From: | Bruce Momjian <bruce(at)momjian(dot)us> |
---|---|
To: | Magnus Hagander <mha(at)sollentuna(dot)net> |
Cc: | 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>, pgsql-patches(at)postgresql(dot)org |
Subject: | Re: [HACKERS] Bad bug in fopen() wrapper code |
Date: | 2006-10-03 01:18:47 |
Message-ID: | 200610030118.k931Iln29956@momjian.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers pgsql-patches |
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 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, is this the right fix?
--
Bruce Momjian bruce(at)momjian(dot)us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Attachment | Content-Type | Size |
---|---|---|
/pgpatches/win_open | text/x-diff | 1022 bytes |
From | Date | Subject | |
---|---|---|---|
Next Message | Jim Nasby | 2006-10-03 01:21:45 | Re: NULL and plpgsql rows |
Previous Message | Bruce Momjian | 2006-10-03 01:17:52 | Re: Patch: Tie stats options to autovacuum in |
From | Date | Subject | |
---|---|---|---|
Next Message | Jim Nasby | 2006-10-03 01:43:01 | Re: Patch: Tie stats options to autovacuum in |
Previous Message | Bruce Momjian | 2006-10-03 01:17:52 | Re: Patch: Tie stats options to autovacuum in |