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 > > > > >>

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

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 negat

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 suppose

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

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))) >

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

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

2006-10-02 Thread Tom Lane
"Magnus Hagander" <[EMAIL PROTECTED]> writes: >> 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 mysel

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