Cleaning up FILE in stdio..

John Baldwin jhb at freebsd.org
Thu Feb 28 13:56:51 UTC 2008


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


More information about the freebsd-arch mailing list