Re: [HACKERS] [PATCHES] Bad bug in fopen() wrapper code

2006-10-03 Thread Zeugswetter Andreas DCP SD
Magnus, is this the right fix? Well, actually msdn states: Return Value If successful, _setmode returns the previous translation mode. A return value of -1 indicates an error So, shouldn't we be testing for -1 instead of 0 ? The thing is probably academic, since _setmode is only supposed

Re: [HACKERS] [PATCHES] Bad bug in fopen() wrapper code

2006-10-03 Thread Tom Lane
Zeugswetter Andreas DCP SD [EMAIL PROTECTED] writes: If successful, _setmode returns the previous translation mode. A return value of -1 indicates an error So, shouldn't we be testing for -1 instead of 0 ? I think the usual convention is to test for 0, unless there are other negative return

Re: [HACKERS] [PATCHES] Bad bug in fopen() wrapper code

2006-10-03 Thread Magnus Hagander
I agree that this code is both wrong and unreadable (although in practice the _setmode will probably never fail, which is why our attention hasn't been drawn to it). Is someone going to submit a patch? I'm hesitant to change the code myself since I'm not in a

Re: [HACKERS] [PATCHES] Bad bug in fopen() wrapper code

2006-10-03 Thread Bruce Momjian
Applied. --- Bruce Momjian wrote: Bruce Momjian wrote: Magnus Hagander wrote: Now, I still twist my head around the lines: if ((fd = _open_osfhandle((long) h, fileFlags O_APPEND)) 0 || (fileFlags

Re: [HACKERS] [PATCHES] Bad bug in fopen() wrapper code

2006-10-02 Thread Magnus Hagander
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))) Without having studied it closely, it might also highlight a bug on failure of the

Re: [HACKERS] [PATCHES] Bad bug in fopen() wrapper code

2006-10-02 Thread Bruce Momjian
Magnus Hagander wrote: 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))) Without having studied it closely, it might also highlight

Re: [HACKERS] [PATCHES] Bad bug in fopen() wrapper code

2006-10-02 Thread Bruce Momjian
Bruce Momjian wrote: Magnus Hagander wrote: 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))) Without having studied it

Re: [HACKERS] [PATCHES] Bad bug in fopen() wrapper code

2006-10-02 Thread Magnus Hagander
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,