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

2007-02-12 Thread Bruce Momjian
I got a private email from Claudio saying he already answered this question: http://archives.postgresql.org/pgsql-patches/2006-09/msg00285.php I forgot I saw it, so I have added comments to the C code. --- bruce

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

2007-02-08 Thread Bruce Momjian
Tom Lane wrote: Magnus Hagander [EMAIL PROTECTED] writes: That is part of the original open() code that Claudio did back for 8.0, so it has definitly been working since then. Hm, maybe best not to touch it, but still... I haven't really read into the code, though... But a qiuck look

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 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 myself since I'm

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-09-30 Thread Tom Lane
Claudio Natoli [EMAIL PROTECTED] writes: 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))) Without having studied it

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

2006-09-27 Thread Magnus Hagander
What's bugging me is that 0 and O_EXCL give the same answer, and O_TRUNC and O_TRUNC | O_EXCL give the same answer, This is ok, as (iirc) O_EXCL only has effect in the presence of O_CREAT. snip more explanation Thanks, Claudio! After looking at the code some more, and actually reading

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

2006-09-27 Thread Claudio Natoli
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

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

2006-09-24 Thread Tom Lane
Magnus Hagander [EMAIL PROTECTED] writes: Attached patch sets the O_CREAT option when appending to files. That looks correct, but I went looking to see if there were any other mistakes of the same ilk, and I'm wondering what the sense is in openFlagsToCreateFileFlags ... seems like it's

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

2006-09-24 Thread Magnus Hagander
Attached patch sets the O_CREAT option when appending to files. That looks correct, but I went looking to see if there were any other mistakes of the same ilk, and I'm wondering what the sense is in openFlagsToCreateFileFlags ... seems like it's ignoring O_EXCL in some combinations and

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

2006-09-24 Thread Tom Lane
Magnus Hagander [EMAIL PROTECTED] writes: That is part of the original open() code that Claudio did back for 8.0, so it has definitly been working since then. Hm, maybe best not to touch it, but still... I haven't really read into the code, though... But a qiuck look doesn't show me any

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

2006-09-24 Thread Tom Lane
Magnus Hagander [EMAIL PROTECTED] writes: This is pretty bad and pretty urgent - with this, systems installed by the MSI installer simply *do not start*, because they are by default configured to write logs to a file... Attached patch sets the O_CREAT option when appending to files.

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

2006-09-24 Thread Claudio Natoli
Hello guys, it's been a while, but... What's bugging me is that 0 and O_EXCL give the same answer, and O_TRUNC and O_TRUNC | O_EXCL give the same answer, This is ok, as (iirc) O_EXCL only has effect in the presence of O_CREAT. (a comment to this effect would help here, as well as perhaps