Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Fri, 19 Dec 2014 16:49:12 -0500
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: possible getopt stderr output changes

On Thu, Dec 11, 2014 at 05:07:56PM -0500, Rich Felker wrote:
> On Wed, Dec 10, 2014 at 07:10:32PM -0500, Rich Felker wrote:
> > The current getopt code uses some ugly write() sequences to generate
> > its output to stderr, and fails to support message translation. The
> > latter was an oversight when locale/translation support was added and
> > should absolutely be fixed. I'm not sure whether we should leave the
> > code using write() though or switch to fprintf.
> 
> It's been pointed out on irc that POSIX requires ferror(stderr) to be
> set if writing the message fails. However fwrite could still be used
> instead of fprintf. If we need to use stdio at all, however, I'd lean
> towards wanting to make the whole write atomic (i.e. hold the lock for
> the whole time) which is more of a pain without fprintf. So basically
> we're looking at:
> 
> fprintf:
> PROS: smaller and simpler code in getopt.c, only one syscall
> CONS: additional ~6.5k of additional code pulled in for static
> 
> fwrite:
> PROS: minimal static linking deps
> CONS: need to use flockfile (or implementation internals) for
> atomicity if desired, and multiple writes (so no atomicity on the fd)

I'm attaching a patch which allows either solution, and the approach
using fwrite is considerably uglier. Even using some stdio internals
directly rather than the public API, the resulting getopt.o is 176
bytes larger than the the fprintf version, but I think the ugliness to
get the required semantics is the worst part. So I'm strongly leaning
towards just using fprintf. The other viable alternative would be
having an internal-use function for printing a variadic list of
strings atomically through stdio, but I still think there's probably
more value in keeping getopt.c independent of musl internals as much
as possible.

Adding support for translation of the messages is a separate step yet
to be done, but the code is setup to support it in either way.

Rich

View attachment "getopt_stdio.diff" of type "text/plain" (1838 bytes)

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.