From: | "Claudio Natoli" <claudio(dot)natoli(at)memetrics(dot)com> |
---|---|
To: | "Magnus Hagander" <mha(at)sollentuna(dot)net>, "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | <pgsql-hackers(at)postgresql(dot)org>, <pgsql-patches(at)postgresql(dot)org> |
Subject: | Re: Bad bug in fopen() wrapper code |
Date: | 2006-09-27 09:07:04 |
Message-ID: | C9A33A2803C7F3479A02A333328A174756CAC0@ewell.memetrics.local |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers pgsql-patches |
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 failure of the _open_osfhandle, obviously)
Cheers,
Claudio
From | Date | Subject | |
---|---|---|---|
Next Message | Markus Schaber | 2006-09-27 09:34:21 | Ignore that mail (was: jar in repository) |
Previous Message | Magnus Hagander | 2006-09-27 08:50:02 | Re: Developer's Wiki |
From | Date | Subject | |
---|---|---|---|
Next Message | Strong, David | 2006-09-27 13:02:46 | Re: Faster StrNCpy |
Previous Message | Jeremy Drake | 2006-09-27 08:57:28 | large object regression tests, take two |