Skip site navigation (1) Skip section navigation (2)

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

From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Magnus Hagander <mha(at)sollentuna(dot)net>, 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 04:00:25
Message-ID: 200610030400.k9340PM21087@momjian.us (view raw or flat)
Thread:
Lists: pgsql-hackerspgsql-patches
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 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?

Claudio sent me some small updates to the patch;  updated version
attached.

-- 
  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: /pgpatches/win_open
Description: text/x-diff (992 bytes)

In response to

Responses

pgsql-hackers by date

Next:From: ITAGAKI TakahiroDate: 2006-10-03 04:02:58
Subject: Re: Another idea for dealing with cmin/cmax
Previous:From: Tom LaneDate: 2006-10-03 01:54:14
Subject: Re: Patch: Tie stats options to autovacuum in

pgsql-patches by date

Next:From: Magnus HaganderDate: 2006-10-03 05:50:27
Subject: Re: [HACKERS] Bad bug in fopen() wrapper code
Previous:From: Tom LaneDate: 2006-10-03 01:54:14
Subject: Re: Patch: Tie stats options to autovacuum in

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group