[Buildroot] [PATCH v2] pppd: Provide error() implementation in pppoe-discovery
Thomas Petazzoni
thomas.petazzoni at free-electrons.com
Fri Oct 28 13:24:19 UTC 2016
Hello,
On Sun, 23 Oct 2016 20:49:58 +0200, stefan.nickl at gmail.com wrote:
> From: Stefan Nickl <Stefan.Nickl at gmail.com>
>
> The pppoe-discovery program calls error() from the CHECK_ROOM macro
> defined in pppoe.h. Since pppoe-discovery is a standalone program not
> linked with the rest of pppd, the only way this could build is by
> linking to glibc's proprietary error(3) function instead of the function
> of the same name (but with different arguments) defined in pppd/utils.c.
>
> So with glibc and uClibc this builds, but will probably crash when the
> assertion is triggered. As the assertion is unlikely to fail, nobody has
> noticed. The build however fails with musl libc since it doesn't
> provide the doppelganger.
What do you mean by doppelganger?
> diff --git a/package/pppd/0002-rp-pppoe-error.patch b/package/pppd/0002-rp-pppoe-error.patch
> new file mode 100644
> index 0000000..f103f4d
> --- /dev/null
> +++ b/package/pppd/0002-rp-pppoe-error.patch
> @@ -0,0 +1,27 @@
> +diff --git a/pppd/plugins/rp-pppoe/pppoe-discovery.c b/pppd/plugins/rp-pppoe/pppoe-discovery.c
> +index 3d3bf4e..55037df 100644
> +--- a/pppd/plugins/rp-pppoe/pppoe-discovery.c
> ++++ b/pppd/plugins/rp-pppoe/pppoe-discovery.c
The patch lacks a description + Signed-off-by.
> +@@ -9,6 +9,7 @@
> + *
> + */
> +
> ++#include <stdarg.h>
> + #include <stdio.h>
> + #include <stdlib.h>
> + #include <unistd.h>
> +@@ -55,6 +56,14 @@ void die(int status)
> + exit(status);
> + }
> +
> ++void error(char *fmt, ...)
> ++{
> ++ va_list pvar;
> ++ va_start(pvar, fmt);
> ++ vfprintf(stderr, fmt, pvar);
> ++ va_end(pvar);
> ++}
This really doesn't seem like the right approach, for several reasons:
- There is already a function with the same name provided in the C
library, which means static linking scenarios will no longer work,
as you'll have clashing symbols.
- The rest of the pppoe-discovery code is using perror(), which is
provided by the C library. So I believe the better fix is to change
the CHECK_ROOM() macro to use the perror() function instead.
Could you rework your patch accordingly?
Thanks a lot,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
More information about the buildroot
mailing list