Re: Patch: incorrect array offset in backend replication tar header

On Thu, Sep 27, 2012 at 6:43 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Brian Weaver <cmdrclueless(at)gmail(dot)com> writes:
>> OK, here is my attempt at patching and correcting the issue in this
>> thread. I have done my best to test to ensure that hot standby,
>> pg_basebackup, and pg_restore of older files work without issues. I
>> think this might be a larger patch that expected, I took some
>> liberties of trying to clean up a bit.
>
>> For example the block size '512' was scattered throughout the code
>> regarding the tar block size. I've replace instances of that with a
>> defined constant TAR_BLOCK_SIZE. I've likewise created other constants
>> and used them in place of raw numbers in what I hope makes the code a
>> bit more readable.
>
> That seems possibly reasonable ...
>
>> I've also used functions like strncpy(), strnlen(), and the like in
>> place of sprintf() where I could. Also instead of using sscanf() I
>> used a custom octal conversion routine which has a hard limit on how
>> many character it will process like strncpy() & strnlen().
>
> ... but I doubt that this really constitutes a readability improvement.
> Or a portability improvement. strnlen for instance is not to be found
> in Single Unix Spec v2 (http://pubs.opengroup.org/onlinepubs/007908799/)
> which is what we usually take as our baseline assumption about which
> system functions are available everywhere. By and large, I think the
> more different system functions you rely on, the harder it is to read
> your code, even if some unusual system function happens to exactly match
> your needs in particular places. It also greatly increases the risk
> of having portability problems, eg on Windows, or non-mainstream Unix
> platforms.
>
> But a larger point is that the immediate need is to fix bugs. Code
> beautification is a separate activity and would be better submitted as
> a separate patch. There is no way I'd consider applying most of this
> patch to the back branches, for instance.
>
> regards, tom lane

Here's a very minimal fix then, perhaps it will be more palatable.
Even though I regret the effort I put into the first patch it's in my
employer's best interest that it's fixed. I'm obliged to try to
remediate the problem in something more acceptable to the community.

enjoy
--

/* insert witty comment here */