Is there a "right" way to deal with error.h inclusions?

Is there a "right" way to deal with error.h inclusions?

[Weldon Goree]

Hi all,

I've been dealing with unconditional inclusions of error.h in a pretty
ad-hoc way in building different things against musl. Does anybody know
of an autohell macro that could be reliably wrapped around error.h
inclusions and its defined functions? (Alternately, any non-auto* based
solution that's worked for people?)

Thanks,
Weldon

Re: Is there a "right" way to deal with error.h inclusions?

[Luca Barbato]

On 29/06/14 14:34, Weldon Goree wrote:
> Hi all,
> 
> I've been dealing with unconditional inclusions of error.h in a pretty
> ad-hoc way in building different things against musl. Does anybody know
> of an autohell macro that could be reliably wrapped around error.h
> inclusions and its defined functions? (Alternately, any non-auto* based
> solution that's worked for people?)

Do you need to check for presence or presence of a symbol in the header?

For autotools AC_EGREP_CPP could work e.g.

AC_EGREP_CPP(error_h_test_succeeded, [
#include <error.h>
#ifdef SYMBOL
error_h_test_succeeded
#endif
])

WAF has a quite nifty check_statement() function.

check_statement('error.h',
	 '#ifndef SYMBOL'
	 '#error'
	 '#endif')


The Libav build system has a check_cpp_condition that works in the same way.

check_cpp_condition error.h "defined SYMBOL"

lu

Re: Is there a "right" way to deal with error.h inclusions?

[Rich Felker]

On Sun, Jun 29, 2014 at 03:47:46PM +0200, Luca Barbato wrote:
> On 29/06/14 14:34, Weldon Goree wrote:
> > Hi all,
> > 
> > I've been dealing with unconditional inclusions of error.h in a pretty
> > ad-hoc way in building different things against musl. Does anybody know
> > of an autohell macro that could be reliably wrapped around error.h
> > inclusions and its defined functions? (Alternately, any non-auto* based
> > solution that's worked for people?)
> 
> Do you need to check for presence or presence of a symbol in the header?

Unlikely; probably the existence or nonexistence of the header, for
which there's a completely standard approach. But I think he was also
asking for a way to do a fallback when they're absent.

Since the issue has come up several times, I'd like it if someone
could explain what all is involved in implementing these functions and
whether they would be candidates for addition to musl.

Of course the best (most portable and easiest at build-time) solution
is simply not to use these functions, or if you're adapting legacy
code that already uses them, simply including an implementation in
your source tree and using it rather than the libc version.

Rich

Re: Is there a "right" way to deal with error.h inclusions?

[Weldon Goree]

On 06/29/2014 07:39 PM, Rich Felker wrote:
> Since the issue has come up several times, I'd like it if someone
> could explain what all is involved in implementing these functions and
> whether they would be candidates for addition to musl.
> 
> Of course the best (most portable and easiest at build-time) solution
> is simply not to use these functions, or if you're adapting legacy
> code that already uses them, simply including an implementation in
> your source tree and using it rather than the libc version.

Personally, as a sysadmin rather than programmer, I'm trying to build
software others have written with the assumption that GNU's error.h and
its defined functions exist (procps-ng was the most recent one, but as
you mentioned this affects several pieces of software).

The ad-hoc bit I mentioned was that I'm usually doing exactly what you
mentioned first: more or less (generally, less or even stub)
implementing the functions that get called (horrifically, often just in
a new <error.h> -- I know, I know...). I should probably just combine
these into an error-compat library, but I was hoping somebody had
already invented that wheel.

Weldon

Re: Is there a "right" way to deal with error.h inclusions?

[Rich Felker]

On Sun, Jun 29, 2014 at 07:55:33PM +0530, Weldon Goree wrote:
> On 06/29/2014 07:39 PM, Rich Felker wrote:
> > Since the issue has come up several times, I'd like it if someone
> > could explain what all is involved in implementing these functions and
> > whether they would be candidates for addition to musl.
> > 
> > Of course the best (most portable and easiest at build-time) solution
> > is simply not to use these functions, or if you're adapting legacy
> > code that already uses them, simply including an implementation in
> > your source tree and using it rather than the libc version.
> 
> Personally, as a sysadmin rather than programmer, I'm trying to build
> software others have written with the assumption that GNU's error.h and
> its defined functions exist (procps-ng was the most recent one, but as
> you mentioned this affects several pieces of software).
> 
> The ad-hoc bit I mentioned was that I'm usually doing exactly what you
> mentioned first: more or less (generally, less or even stub)
> implementing the functions that get called (horrifically, often just in
> a new <error.h> -- I know, I know...). I should probably just combine
> these into an error-compat library, but I was hoping somebody had
> already invented that wheel.

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.

Rich

Re: Is there a "right" way to deal with error.h inclusions?

[Szabolcs Nagy]

[-- Attachment #1: Type: text/plain, Size: 371 bytes --]

* Rich Felker <dalias@libc.org> [2014-06-29 11:43:57 -0400]:
> 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.

i attached the thing i got last time i looked into this
(there is global state accessed without sync and other ugliness)

[-- Attachment #2: error.h --]
[-- Type: text/x-chdr, Size: 234 bytes --]

extern void (*error_print_progname)(void);
extern unsigned int error_message_count;
extern int error_one_per_line;

void error(int, int, const char *, ...);
void error_at_line(int, int, const char *, unsigned int, const char *, ...);

[-- Attachment #3: error.c --]
[-- Type: text/x-csrc, Size: 1166 bytes --]

#define _GNU_SOURCE
#include <stdarg.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "error.h"

extern char *__progname;

void (*error_print_progname)(void) = 0;
unsigned int error_message_count = 0;
int error_one_per_line = 0;

static void eprint(int status, int e, const char *file, unsigned int line, const char *fmt, va_list ap)
{
	if (file && error_one_per_line) {
		static const char *oldfile;
		static unsigned int oldline;
		if (line == oldline && strcmp(file, oldfile) == 0)
			return;
		oldfile = file;
		oldline = line;
	}
	if (error_print_progname)
		error_print_progname();
	else
		fprintf(stderr, "%s: ", __progname);
	if (file)
		fprintf(stderr, "%s:%u: ", file, line);
	vfprintf(stderr, fmt, ap);
	if (e)
		fprintf(stderr, ": %s\n", strerror(e));
	error_message_count++;
	if (status)
		exit(status);
}

void error(int status, int e, const char *fmt, ...)
{
	va_list ap;
	va_start(ap,fmt);
	eprint(status, e, 0, 0, fmt, ap);
	va_end(ap);
}

void error_at_line(int status, int e, const char *file, unsigned int line, const char *fmt, ...)
{
	va_list ap;
	va_start(ap,fmt);
	eprint(status, e, file, line, fmt, ap);
	va_end(ap);
}

Re: Is there a "right" way to deal with error.h inclusions?

[Rich Felker]

On Sun, Jun 29, 2014 at 07:21:58PM +0200, Szabolcs Nagy wrote:
> * Rich Felker <dalias@libc.org> [2014-06-29 11:43:57 -0400]:
> > 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.
> 
> i attached the thing i got last time i looked into this
> (there is global state accessed without sync and other ugliness)

Yes, there's also the fact that the error count could wrap and wrongly
indicate "no errors" to the program (presumably it should saturate),
and lack of atomicity of writes.

I'd kinda lean towards not supporting error_print_progname and
error_one_per_line at all. Then the whole output can be accomplished
with a single dprintf, which is much safer (if someone does stupidly
use this interface in program-in-corrupted-state situations) than
using stdio.

Of course it might be better to just put all this ugliness in a
third-party liberror.a.

Rich

Re: Is there a "right" way to deal with error.h inclusions?

[u-igbb]

On Sun, Jun 29, 2014 at 03:30:15PM -0400, Rich Felker wrote:
> Of course it might be better to just put all this ugliness in a
> third-party liberror.a.

+1

Rune

Re: Is there a "right" way to deal with error.h inclusions?

[Weldon Goree]

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 */

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.

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...)
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.

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). 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.

Weldon

Re: Is there a "right" way to deal with error.h inclusions?

[Rich Felker]

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

Re: Is there a "right" way to deal with error.h inclusions?

[Weldon Goree]

On 06/30/2014 12:41 AM, Rich Felker wrote:
>>
>> void (*error_print_progname) (void) /* always supplied by the caller */
> 
> What is this one supposed to do?
> 

It's caller-supplied, so whatever the caller wants; the caller assigns
it the address of some function he wrote. Internally, if it's non-null
it suppresses the printing of char *program_name, followed by a colon
and space, before the formatted message.

> 
> 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.

Fair enough; I guess the fact that error_message_count can be
incremented implies that fatal errors aren't what these two functions
are for.

Weldon