Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 1 Oct 2012 08:16:31 -0400
From:      John Baldwin <jhb@freebsd.org>
To:        Konstantin Belousov <kostikbel@gmail.com>
Cc:        arch@freebsd.org
Subject:   Re: stdio and short file descriptors revisited
Message-ID:  <201210010816.31511.jhb@freebsd.org>
In-Reply-To: <20121001092113.GH35915@deviant.kiev.zoral.com.ua>
References:  <201209281847.39663.jhb@freebsd.org> <20121001092113.GH35915@deviant.kiev.zoral.com.ua>

next in thread | previous in thread | raw e-mail | index | archive | help
On Monday, October 01, 2012 5:21:13 am Konstantin Belousov wrote:
> On Fri, Sep 28, 2012 at 06:47:39PM -0400, John Baldwin wrote:
> > Four years or so ago I cleaned up some of the stdio internals as fallout from 
> > running into problems with stdio using a short instead of an int to hold file 
> > descriptors.  Back then I got sidetracked with attempting to make FILE opaque 
> > and ended up never getting around to bumping _file from a short to an int.  I 
> > recently ran back into the SHRT_MAX limit at work again and came up with a 
> > patch to fix this.
> > 
> > To preserve the ABI, it is necessary to leave the existing short _file in 
> > place and add a new int _file to the end of the FILE structure.  Also, for old 
> > applications, the old _file (_ofile in the patch) must still be valid.  The 
> > approach I have taken is to bump the symbol version for routines that create 
> > FILE objects with a non-fake _file (fopen, fdopen, and freopen).  The old 
> > FBSD_1.0 variants still fail if an fd is greater than SHRT_MAX (and thus 
> > cannot be safely stored in _ofile).  The new FBSD_1.3 variants assign to both 
> > _file and _ofile if the fd is less than SHRT_MAX.  I also changed fileno()
> > to no longer be an inline macro in <stdio.h> but to always be a function call 
> > going forward.
> > 
> > If folks think this is ok, I'll hack up a modified version that hides _file
> > from outside consumers (rename it to _nfile or some such) and send it for a
> > ports-exp run before committing to make sure there aren't any 3rd party apps
> > accessing _file directly.
> > 
> > http://www.FreeBSD.org/~jhb/patches/stdio_file.patch
> 
> The corner case left unhandled is the situation where we have a dso which
> is linked against FBSD_1.0 version of libc, but which gets FILE * as an
> API argument for some of its exported routines. If the implementation
> uses fileno(3), it would fail. I have no idea how to fix this, most
> likely, the issue is not fixable at all. Workaround seems to be to force
> the __isthreaded to 1. Might be, as an ugly hack, some flag could be
> added to the stdbuf(1), if anybody cares enough.
> 
> Otherwise, the patch looks good.

For things that layer on top of, say, funopen() (like libftpio, etc.),
fileno() is meaningless.  However, if libfoo.so calls fopen() and returns
it to the caller, then yes, that would break in that if the opened fd was
greater than 2^15 fileno() would return -1, not the real fd.  At least it
would return a known bogus fd such that attempts to use it should fail with
EBADF rather than affecting some other unrelated fd.  In practice I am not
sure how many applications that use stdio really expect to be able to use
fileno().  The two production cases I've seen that have run into the 2^15
limit haven't used fileno() at all, the breakage they see is that fopen()
fails.

Which is to say, I don't think that edge case will occur in practice.  I
do agree that it would be possible to add a workaround (such as forcing
__isthreaded to true in the compat f*open() shims) in the future if it is
really needed.

-- 
John Baldwin



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