From: | "Magnus Hagander" <mha(at)sollentuna(dot)net> |
---|---|
To: | "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, <pgsql-hackers(at)postgresql(dot)org>, "Claudio Natoli" <claudio(dot)natoli(at)memetrics(dot)com> |
Cc: | <pgsql-patches(at)postgresql(dot)org> |
Subject: | Re: [HACKERS] Bad bug in fopen() wrapper code |
Date: | 2006-10-02 12:19:08 |
Message-ID: | 6BCB9D8A16AC4241919521715F4D8BCEA35766@algol.sollentuna.se |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers pgsql-patches |
> >> 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 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)
>
> 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
> position to test it.
I can look at fixing that. Is it something we want to do for 8.2, or
wait until 8.3? If there's a hidden bug, I guess 8.2?
//Magnus
From | Date | Subject | |
---|---|---|---|
Next Message | José Orlando Pereira | 2006-10-02 12:44:53 | Re: Replication hooks discussion |
Previous Message | Hannu Krosing | 2006-10-02 09:59:26 | Re: Another idea for dealing with cmin/cmax |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2006-10-02 14:33:18 | Re: [HACKERS] Bad bug in fopen() wrapper code |
Previous Message | Zdenek Kotala | 2006-10-02 07:53:45 | Re: [PATCHES] Generic Monitoring Framework with DTrace patch |