Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 28 Feb 2008 08:08:35 -0500
From:      John Baldwin <jhb@freebsd.org>
To:        Peter Jeremy <peterjeremy@optushome.com.au>
Cc:        arch@freebsd.org
Subject:   Re: Cleaning up FILE in stdio..
Message-ID:  <200802280808.35678.jhb@freebsd.org>
In-Reply-To: <20080228083555.GX83599@server.vk2pj.dyndns.org>
References:  <200802262251.m1QMp7bV021709@hergotha.csail.mit.edu> <200802271138.33979.jhb@freebsd.org> <20080228083555.GX83599@server.vk2pj.dyndns.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Thursday 28 February 2008 03:35:55 am Peter Jeremy wrote:
> On Wed, Feb 27, 2008 at 11:38:33AM -0500, John Baldwin wrote:
> >> You could change _file from 'short' to 'unsigned short' without breaking
> >> the ABI - this would allow either 65535 or 65536 file descriptors (I'm
> >> not sure whether _file == -1 is special or not).  This would postpone
> >> the problem for some time.
> >
> >-1 is used a lot in the stdio code for file's not backed by an fd.  My 
problem 
> >though is that this doesn't help with existing binaries that are already 
> >compiled (which is what I have to deal with).  Had fileno() not been 
inlined 
> >I would have been ok, but that's pretty much done for me as far as my 
current 
> >problem on 6.x.  Had I just been able to change FILE * and not had inlines, 
> >then a new fopen would have worked fine in my case.
> 
> My suggestion was based on short and ushort having the same size and
> (short)-1 and (ushort)65535 having the same bit pattern.  Any code
> accessing fileno() should not be checking or validating the result
> but just passing it to low-level I/O routines.  This would provide the
> following:
>                Existing code     New code
> 		   short          ushort
>     FD         sign-extended   zero-extended
>    -1              -1           65535
>     0..32767        0..32767        0..32767
> 32768..65534   -32768..-2 [*]   32768..65534
>  >65534          EMFILE            EMFILE
> 
> [*] This could potentially be fixed using libc or kernel shims.

Actually, a correction on my part: ushort would double the range ok in my case 
as gethostbyname() doesn't use fileno(), and the new fread()/fclose() would 
work ok in this case.

> >think we can get away with renaming '_file' to '_ofile' and adding a 
new 'int 
> >_file' at the bottom of the struct and making sure '_ofile' is always in 
sync 
> >(when possible, truncated when _file is too bug).
> 
> Truncation opens up the possibility that old executables could fopen()
> lots of files (without getting any indication of a problem) and then
> use fileno() to reference a truncated _ofile - causing it to access
> some totally unrelated file.  Admittedly, that is no worse than would
> happen today.

I had considered that, but I think it is an acceptable tradeoff considering 
that 1) the apps would already be broken today in this instance, and 2) you 
have to use fileno(3) to get a problem.  If you just use 
fopen()/fread()/fclose() then it will work fine (even on old apps that aren't 
recompiled).

> >Also, I think we can do the new _file in HEAD for 8.0 w/o any worries.  I 
> >don't think waiting until 9.0 buys anything there.
> 
> I was thinking in terms of changing _file to int without backward
> compatibility.  I'm not sure that that could be done for 8.0 (though,
> as you have pointed out, it can't be done at all).

It can if we accept the truncation issue.

-- 
John Baldwin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200802280808.35678.jhb>