Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sun, 29 Jun 2014 15:11:57 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: Is there a "right" way to deal with error.h inclusions?

On Sun, Jun 29, 2014 at 11:16:18PM +0530, Weldon Goree wrote:
> On 06/29/2014 09:13 PM, Rich Felker wrote:
> > Yes, I know that's a pain. Again, I'm open to considering adding
> > compatible functions to musl, but first I need someone with an
> > interest in it to do the research on what's involved.
> 
> Well, for error.h, it declares (this is from glibc2.19, the only
> one I have the tarball of at the moment):
> 
> void error(int status, int errnum, const char *message, ...) [attributed
> as printf-style]
> 
> void error_at_line(int status, int errnum, const char *filename,
> unsigned int lineno, const char *message, ...) [attributed as printf-style]
> 
> void (*error_print_progname) (void) /* always supplied by the caller */

What is this one supposed to do?

> unsigned int error_message_count
> 
> int error_one_per_line
> 
> A STUB SOLUTION:
> Make a new error.h that just no-ops those functions. I've done that a
> few times but even that gets non-trivial because error() is weakly
> aliased to __error(), and there are plenty of functions named error()
> floating around out there...
> 
> But, sometimes I do want (some of) the semantics of the error call,
> which leads to the issue of a non-stub solution.

Yeah, I think a stub is probably more of a potential problem than a
benefit, since programs that detect and use these functions might
assume they actually work and fail to report errors if they don't.

> NOTES FOR A NON-STUB SOLUTION:
> 
> error() does the following:
> 
> 1. sets the thread to uncancellable
> 2. flushes stdout
> 3. flocks stderr
> 4. calls (*error_print_progname)(), if non-NULL
> 5. printfs extern char *program_name, which is assumed to be defined
> 6. Converts *message into wchar_t *wmessage (that's right... it calls
> alloca() and realloc() after you've already told it that it's in error...)

I think (hope) these functions are not intended for use when the
program is crashing, but rather for reporting things like bad input or
resource-related failures that are not the result of programming
errors but legitimate conditions that could occur at runtime. The
existence of the error_at_line function supports this intended usage:
it would be used when parsing files to report the location of a
problem in the file being parsed.

> 7. printfs whatever the caller puts in the format string (that's the
> const char* in the arguments)
> 8. also formats the status and error number (which are the first two
> int's in the arguments)
> 9. frees wmessage
> 10. increments error_message_count
> 11. funlocks stderr
> 12. sets the thread to cancellable
> 
> error_at_line does the same, with a supplied file name and line number
> also printf'd out as part of the message; additionally, steps 4 through
> 8 are only performed if error_one_per_line is 0 || there was not
> previously an error called on that line.
> 
> error_message_count (formerly known as warning_cntr) does leak out into
> a few other places in the library; the file parsing routine in
> nscd_conf.c returns a failure if error_message_count is different from
> what error_message_count was when the function began (that seems like an
> awful idea). Also, gencat() in catgets returns error_message_count != 0.
> I have not, however, yet run across a piece of external software that
> looks at error_message_count or error_one_per_line, so they can
> (probably) be ignored.

nscd and gencat are programs shipped with glibc, not part of the
library.

> Frankly, I don't *like* either of those functions (I don't think a
> thread already known to be in error should be doing things like setting
> itself uncancellable or incrementing a static variable).

Like I said above, I think you misunderstand what "in error" means
here. And blocking cancellation is simply standard (and necessary)
practice whenever you'll be calling multiple functions which are
cancellation points and cancellation midway would be problematic.
Otherwise a partial but incomplete side effects would be observed on
cancellation, and without cleanup handlers, memory may leak, etc.

> I definitely
> don't like the idea of the message-printing routine allocating both heap
> and stack memory. When I've gone past a stub I've usually just passed
> the char *message and va_args to printf(), and I think an API/ABI
> compatible version could do basically that.

I agree that the allocation is very poor behavior and I see no reason
it should be necessary.

Rich

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.