Re: pg_archivecleanup bug

On 03/19/2014 02:30 AM, Bruce Momjian wrote:
> On Tue, Mar 18, 2014 at 09:13:28PM +0200, Heikki Linnakangas wrote:
>> On 03/18/2014 09:04 PM, Simon Riggs wrote:
>>> On 18 March 2014 18:55, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> wrote:
>>>
>>>> That said, I don't find comma expression to be particularly "not
>>>> simple".
>>>
>>> Maybe, but we've not used it before anywhere in Postgres, so I don't
>>> see a reason to start now. Especially since C is not the native
>>> language of many people these days and people just won't understand
>>> it.
>>
>> Agreed. The psqlODBC code is littered with comma expressions, and
>> the first time I saw it, it took me a really long time to figure out
>> what "if (foo = malloc(...), foo) { } " meant. And I consider myself
>> quite experienced with C.
>
> I can see how the comma syntax would be confusing, though it does the
> job well. Attached is a patch that does the double-errno. However,
> some of these loops are large, and there are 'continue' calls in there,
> causing the addition of many new errno locations. I am not totally
> comfortable that this new coding layout will stay unbroken.
>
> Would people accept?
>
> for (errno = 0; (dirent = readdir(dir)) != NULL; errno = 0)
>
> That would keep the errno's together and avoid the 'continue' additions.

That's clever. A less clever way would be:

for (;;)
{
errno = 0;
if ((dirent = readdir(dir)) != NULL)
break;

...
}

I'm fine with either, but that's how I would naturally write it.

Yet another way would be to have a wrapper function for readdir that
resets errno, and just replace the current readdir() calls with that.

And now that I look at initdb.c, walkdir is using the comma expression
for this already. So there's some precedence, and it doesn't actually
look that bad. So I withdraw my objection for that approach; I'm fine
with any of the discussed alternatives, really.

- Heikki