Bug 21008

Summary: Incompatible with MUSL libc: error.h and error() not provided
Product: elfutils Reporter: Luiz Angelo Daros de Luca <luizluca>
Component: generalAssignee: Not yet assigned to anyone <unassigned>
Status: RESOLVED FIXED    
Severity: normal CC: elfutils-devel, mark, ross
Priority: P2    
Version: unspecified   
Target Milestone: ---   
Host: Target:
Build: Last reconfirmed:
Bug Depends on:    
Bug Blocks: 21002    

Description Luiz Angelo Daros de Luca 2016-12-30 20:42:06 UTC
Hello,

From original bug 21002:

- The idea of using system.h to define error () seems fine. But the current replacement doesn't seem complete. error () also flushes stdout first. Which we don't really seem to use, so might not be essential. But it also calls exit (status) if status != 0. Which is essential to how error is used in elfutils. Also error () updates error_message_count which is used in a couple of cases to know what exit code a program should use (if error itself didn't exit).

The proposed solution would be to:

1) move error.h inclusion into system.h and conditionally include either error.h or err.h
2) build a good replacement for error()

The only problem is #2. 

This thread is about this problem: http://marc.info/?l=musl&m=140404527111778&w=2

The solution that was already present in OpenWRT/LEDE was simply to use:

#define error(status, errno, ...) err(status, __VA_ARGS__)

Which is not enough as error replacement. It always exits even when status is 0, it does not flush stdout and it does not increase the (absent) error_message_count.

I didn't noticed the missing error_message_count as a problem because, for OpenWRT/LEDE, only elfutils libs where compiled (and they don't use error_message_count). So, for my use case, this might be enough:

#define error(status, errno, ...) \
   fflush(stdout); \
   warn(__VA_ARGS__); \ // == err(status,...) without exit
   if (errno) exit(errno);

There is a minor problem of ignoring errno, which might be considered an "optional feature".

For a full built, the problem is error_message_count. It is a global variable and it must be initialized. Probably the best solution would be to create a simplified error.h/error.c, which exports error and error_message_count and gets optionally build and linked (autoconf) into the generated files when error.h is absent.

Some existing approaches so far:

https://github.com/GregorR/musl-cross/blob/master/patches/elfutils-no_error.patch (original inspiration)
https://patchwork.openembedded.org/patch/111665/
Comment 1 Luiz Angelo Daros de Luca 2016-12-30 21:16:24 UTC
BTW, simply moving the include into system.h will dramatically reduce the number of files I need to patch in order to compile with MUSL.
Comment 2 Ross Burton 2018-07-04 14:54:43 UTC
I've just sent a patch to the list to consolidate the error includes.
Comment 3 Mark Wielaard 2021-08-27 17:13:54 UTC
commit 76c84c137a82a7cacbc69b1696052491b3bb81cb
Author: Saleem Abdulrasool <abdulras@google.com>
Date:   Fri Aug 20 20:28:23 2021 +0000

    handle libc implementations which do not provide `error.h`
    
    Introduce a configure time check for the presence of `error.h`.  In the
    case that `error.h` is not available, we can fall back to `err.h`.
    Although `err.h` is not a C standard header (it is a BSD extension),
    many libc implementations provide.  If there are targets which do not
    provide an implementation of `err.h`, it would be possible to further
    extend the implementation to be more portable.
    
    This resolves bug #21008.
    
    Signed-off-by: Saleem Abdulrasool <abdulras@google.com>

commit 4d6dd0e5ad5c3366cbf701b4fb62b6d91be545f8
Author: Saleem Abdulrasool <abdulras@google.com>
Date:   Fri Aug 27 15:51:47 2021 +0000

    lib: avoid potential problems with `-fno-common`
    
    This properly homes the fallback function into a translation unit rather
    than trying to define an inline common definition for the fallback path.
    The intent of the original approach was to actually simply avoid adding
    a new source file that is used for the fallback path.  However, that may
    cause trouble with multiple definitions if the symbol does not get vague
    linkage (which itself is not particularly great).  This simplifies the
    behaviour at the cost of an extra inode.

commit 610623458b7e98ed3e912e4b7ca8050f6ce4c698
Author: Mark Wielaard <mark@klomp.org>
Date:   Fri Aug 27 18:47:30 2021 +0200

    Add lib/error.c
    
    This new file was supposed to be part of 4d6dd0e5a "lib: avoid potential
    problems with `-fno-common`".
    
    Signed-off-by: Mark Wielaard <mark@klomp.org>