Re: Patch: incorrect array offset in backend replication tar header - Mailing list pgsql-hackers

From Brian Weaver
Subject Re: Patch: incorrect array offset in backend replication tar header
Date
Msg-id CAAhXZGuEi9W3kEyZh0-pdnWw_hU1c3NrTvyQxrH2YM=N_4keGg@mail.gmail.com
Whole thread Raw
In response to Re: Patch: incorrect array offset in backend replication tar header  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Patch: incorrect array offset in backend replication tar header  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
On Thu, Sep 27, 2012 at 6:43 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Brian Weaver <cmdrclueless@gmail.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 */

Attachment

pgsql-hackers by date:

Previous
From: Euler Taveira
Date:
Subject: Re: Switching timeline over streaming replication
Next
From: Peter Eisentraut
Date:
Subject: setting per-database/role parameters checks them against wrong context