Re: [PATCHES] Bad bug in fopen() wrapper code - Mailing list pgsql-hackers

From Claudio Natoli
Subject Re: [PATCHES] Bad bug in fopen() wrapper code
Date
Msg-id C9A33A2803C7F3479A02A333328A174756CAC0@ewell.memetrics.local
Whole thread Raw
Responses Re: [PATCHES] Bad bug in fopen() wrapper code  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
Magnus Hagander writes:
> Now, I still twist my head around the lines:
>     if ((fd = _open_osfhandle((long) h, fileFlags & O_APPEND)) < 0
> ||
>         (fileFlags & (O_TEXT | O_BINARY) && (_setmode(fd,
> fileFlags & (O_TEXT | O_BINARY)) < 0)))
>
>
> With the _setmode() call deep in the if statement... I would suggest we
> split that up into a couple of lines to make it more readable - I'm sure
> all compilers will easily optimise it into the same code anyway.
> Reasonable?

I agree it would be clearer if split up.

Without having studied it closely, it might also highlight a bug on failure of the second clause -- if the _setmode
fails,shouldn't _close be called instead of CloseHandle, and -1 returned?  (CloseHandle would still be called on
failureof the _open_osfhandle, obviously) 

Cheers,
Claudio

pgsql-hackers by date:

Previous
From: "Magnus Hagander"
Date:
Subject: Re: Developer's Wiki
Next
From: Markus Schaber
Date:
Subject: Ignore that mail (was: jar in repository)