All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] libnl: musl fixes
@ 2016-08-11 14:06 André Draszik
  2016-08-12  5:45 ` Khem Raj
  2016-08-19 10:31 ` André Draszik
  0 siblings, 2 replies; 10+ messages in thread
From: André Draszik @ 2016-08-11 14:06 UTC (permalink / raw)
  To: openembedded-core

The libnl+musl combination has two issues at the moment:
a) a public header file #include's an incorrect file
   (sys/poll.h instead of poll.h), which causes warnings and
   potentially errors (-Werror) in a musl based system
b) musl only ever provides the XSI version of strerror_r()

a) can be fixed using a generic patch
b) has to be applied on demand for musl-based systems, only

Signed-off-by: André Draszik <git@andred.net>
---
 ...nclude-standard-poll.h-instead-of-sys-pol.patch |  40 +++++++
 .../libnl/0002-musl-fix-strerror_r-usage.patch     | 124 +++++++++++++++++++++
 meta/recipes-support/libnl/libnl_3.2.25.bb         |   2 +
 3 files changed, 166 insertions(+)
 create mode 100644 meta/recipes-support/libnl/libnl/0001-netlink.h-include-standard-poll.h-instead-of-sys-pol.patch
 create mode 100644 meta/recipes-support/libnl/libnl/0002-musl-fix-strerror_r-usage.patch

diff --git a/meta/recipes-support/libnl/libnl/0001-netlink.h-include-standard-poll.h-instead-of-sys-pol.patch b/meta/recipes-support/libnl/libnl/0001-netlink.h-include-standard-poll.h-instead-of-sys-pol.patch
new file mode 100644
index 0000000..c9096ce
--- /dev/null
+++ b/meta/recipes-support/libnl/libnl/0001-netlink.h-include-standard-poll.h-instead-of-sys-pol.patch
@@ -0,0 +1,40 @@
+From 7f73d736db539eb4c6e7d59ed9332615d10ea785 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Andr=C3=A9=20Draszik?= <git@andred.net>
+Date: Thu, 11 Aug 2016 11:53:44 +0100
+Subject: [PATCH 1/2] netlink.h: #include standard poll.h instead of sys/poll.h
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+musl issues a #warning if a non-standard file like is included
+instead of the posix version (sys/poll.h vs poll.h in this
+case).
+Given netlink.h is installed and intended to be used by other
+projects, those other projects fail to compile if they enable
+-Werror (e.g. crda).
+
+Just do the correct thing and #include <poll.h> (the file
+specified by posix).
+
+Signed-off-by: André Draszik <git@andred.net>
+---
+Upstream-Status: Pending
+ include/netlink/netlink.h | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/include/netlink/netlink.h b/include/netlink/netlink.h
+index 28dba06..61656b3 100644
+--- a/include/netlink/netlink.h
++++ b/include/netlink/netlink.h
+@@ -16,7 +16,7 @@
+ #include <stdint.h>
+ #include <string.h>
+ #include <stdlib.h>
+-#include <sys/poll.h>
++#include <poll.h>
+ #include <sys/socket.h>
+ #include <sys/types.h>
+ #include <sys/time.h>
+-- 
+2.8.1
+
diff --git a/meta/recipes-support/libnl/libnl/0002-musl-fix-strerror_r-usage.patch b/meta/recipes-support/libnl/libnl/0002-musl-fix-strerror_r-usage.patch
new file mode 100644
index 0000000..3ded5b1
--- /dev/null
+++ b/meta/recipes-support/libnl/libnl/0002-musl-fix-strerror_r-usage.patch
@@ -0,0 +1,124 @@
+From 4a88a12742ffc0a3ba3b4d4bd1398fa278ab1738 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Andr=C3=A9=20Draszik?= <git@andred.net>
+Date: Thu, 11 Aug 2016 13:15:23 +0100
+Subject: [PATCH 2/2] musl: fix strerror_r() usage
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+glibc provides two versions of strerror_r(), which
+can be chosen between using feature test macros
+_GNU_SOURCE and _POSIX_C_SOURCE.
+musl on the other hand only ever provides the posix
+version of strerror_r().
+
+While this patch is not switching to the XSI version
+of strerror_r() in general, all implementations of
+the XSI version I have checked (bionic, uclibc,
+musl, glibc), fill buf with as many bytes as possible
+and NUL-terminate buf correctly.
+
+Switch to using XSI semantics. (This patch is only
+intended to be applied when compiling against
+musl - unfortunately musl doesn't provide any
+preprocessor directives to detect use of musl.)
+
+Signed-off-by: André Draszik <git@andred.net>
+---
+Upstream-Status: Inappropriate [musl specific]
+ lib/fib_lookup/lookup.c | 5 ++++-
+ lib/handlers.c          | 3 ++-
+ lib/msg.c               | 3 ++-
+ lib/route/route_obj.c   | 3 ++-
+ src/lib/utils.c         | 6 ++++--
+ 5 files changed, 14 insertions(+), 6 deletions(-)
+
+diff --git a/lib/fib_lookup/lookup.c b/lib/fib_lookup/lookup.c
+index 3d07317..490f04e 100644
+--- a/lib/fib_lookup/lookup.c
++++ b/lib/fib_lookup/lookup.c
+@@ -125,6 +125,9 @@ static void result_dump_line(struct nl_object *obj, struct nl_dump_params *p)
+ {
+ 	struct flnl_result *res = (struct flnl_result *) obj;
+ 	char buf[256];
++	char buf2[256];
++
++	strerror_r(-res->fr_error, buf2, sizeof(buf2));
+ 
+ 	nl_dump_line(p, "table %s prefixlen %u next-hop-selector %u\n",
+ 		rtnl_route_table2str(res->fr_table_id, buf, sizeof(buf)),
+@@ -133,7 +136,7 @@ static void result_dump_line(struct nl_object *obj, struct nl_dump_params *p)
+ 		     nl_rtntype2str(res->fr_type, buf, sizeof(buf)));
+ 	nl_dump(p, "scope %s error %s (%d)\n",
+ 		rtnl_scope2str(res->fr_scope, buf, sizeof(buf)),
+-		strerror_r(-res->fr_error, buf, sizeof(buf)), res->fr_error);
++		buf2, res->fr_error);
+ }
+ 
+ static void result_dump_details(struct nl_object *obj, struct nl_dump_params *p)
+diff --git a/lib/handlers.c b/lib/handlers.c
+index a6a97bb..1b9bb4f 100644
+--- a/lib/handlers.c
++++ b/lib/handlers.c
+@@ -81,8 +81,9 @@ static int nl_error_handler_verbose(struct sockaddr_nl *who,
+ 	FILE *ofd = arg ? arg : stderr;
+ 	char buf[256];
+ 
++	strerror_r(-e->error, buf, sizeof(buf));
+ 	fprintf(ofd, "-- Error received: %s\n-- Original message: ",
+-		strerror_r(-e->error, buf, sizeof(buf)));
++		buf);
+ 	print_header_content(ofd, &e->msg);
+ 	fprintf(ofd, "\n");
+ 
+diff --git a/lib/msg.c b/lib/msg.c
+index bcf1aa8..eec2833 100644
+--- a/lib/msg.c
++++ b/lib/msg.c
+@@ -916,8 +916,9 @@ static void dump_error_msg(struct nl_msg *msg, FILE *ofd)
+ 		char buf[256];
+ 		struct nl_msg *errmsg;
+ 
++		strerror_r(-err->error, buf, sizeof(buf));
+ 		fprintf(ofd, "    .error = %d \"%s\"\n", err->error,
+-			strerror_r(-err->error, buf, sizeof(buf)));
++			buf);
+ 		fprintf(ofd, "  [ORIGINAL MESSAGE] %zu octets\n", sizeof(*hdr));
+ 
+ 		errmsg = nlmsg_inherit(&err->msg);
+diff --git a/lib/route/route_obj.c b/lib/route/route_obj.c
+index dd4bf49..6fc4321 100644
+--- a/lib/route/route_obj.c
++++ b/lib/route/route_obj.c
+@@ -257,9 +257,10 @@ static void route_dump_details(struct nl_object *a, struct nl_dump_params *p)
+ 	}
+ 
+ 	if ((r->ce_mask & ROUTE_ATTR_CACHEINFO) && r->rt_cacheinfo.rtci_error) {
++		strerror_r(-r->rt_cacheinfo.rtci_error, buf, sizeof(buf));
+ 		nl_dump_line(p, "    cacheinfo error %d (%s)\n",
+ 			r->rt_cacheinfo.rtci_error,
+-			strerror_r(-r->rt_cacheinfo.rtci_error, buf, sizeof(buf)));
++			buf);
+ 	}
+ 
+ 	if (r->ce_mask & ROUTE_ATTR_METRICS) {
+diff --git a/src/lib/utils.c b/src/lib/utils.c
+index e5eacde..d9cc7a8 100644
+--- a/src/lib/utils.c
++++ b/src/lib/utils.c
+@@ -79,8 +79,10 @@ void nl_cli_fatal(int err, const char *fmt, ...)
+ 		vfprintf(stderr, fmt, ap);
+ 		va_end(ap);
+ 		fprintf(stderr, "\n");
+-	} else
+-		fprintf(stderr, "%s\n", strerror_r(err, buf, sizeof(buf)));
++	} else {
++		strerror_r(err, buf, sizeof(buf));
++		fprintf(stderr, "%s\n", buf);
++	}
+ 
+ 	exit(abs(err));
+ }
+-- 
+2.8.1
+
diff --git a/meta/recipes-support/libnl/libnl_3.2.25.bb b/meta/recipes-support/libnl/libnl_3.2.25.bb
index cabe841..540b019 100644
--- a/meta/recipes-support/libnl/libnl_3.2.25.bb
+++ b/meta/recipes-support/libnl/libnl_3.2.25.bb
@@ -13,7 +13,9 @@ DEPENDS = "flex-native bison-native"
 SRC_URI = "http://www.infradead.org/~tgr/${BPN}/files/${BP}.tar.gz \
            file://fix-pktloc_syntax_h-race.patch \
            file://fix-pc-file.patch \
+           file://0001-netlink.h-include-standard-poll.h-instead-of-sys-pol.patch \
           "
+SRC_URI_append_libc-musl = " file://0002-musl-fix-strerror_r-usage.patch"
 
 SRC_URI[md5sum] = "03f74d0cd5037cadc8cdfa313bbd195c"
 SRC_URI[sha256sum] = "8beb7590674957b931de6b7f81c530b85dc7c1ad8fbda015398bc1e8d1ce8ec5"
-- 
2.8.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] libnl: musl fixes
  2016-08-11 14:06 [PATCH] libnl: musl fixes André Draszik
@ 2016-08-12  5:45 ` Khem Raj
  2016-08-12  7:26   ` André Draszik
  2016-08-19 10:31 ` André Draszik
  1 sibling, 1 reply; 10+ messages in thread
From: Khem Raj @ 2016-08-12  5:45 UTC (permalink / raw)
  To: André Draszik; +Cc: openembedded-core

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


> On Aug 11, 2016, at 7:06 AM, André Draszik <git@andred.net> wrote:
> 
> The libnl+musl combination has two issues at the moment:
> a) a public header file #include's an incorrect file
>   (sys/poll.h instead of poll.h), which causes warnings and
>   potentially errors (-Werror) in a musl based system
> b) musl only ever provides the XSI version of strerror_r()
> 

yes so you can check for something like
(_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && ! _GNU_SOURCE
use XSI version
else use GNU version

perf might also use similar patch.

> a) can be fixed using a generic patch
> b) has to be applied on demand for musl-based systems, only

if above is doable then this is not needed.

> 
> Signed-off-by: André Draszik <git@andred.net>
> ---
> ...nclude-standard-poll.h-instead-of-sys-pol.patch |  40 +++++++
> .../libnl/0002-musl-fix-strerror_r-usage.patch     | 124 +++++++++++++++++++++
> meta/recipes-support/libnl/libnl_3.2.25.bb         |   2 +
> 3 files changed, 166 insertions(+)
> create mode 100644 meta/recipes-support/libnl/libnl/0001-netlink.h-include-standard-poll.h-instead-of-sys-pol.patch
> create mode 100644 meta/recipes-support/libnl/libnl/0002-musl-fix-strerror_r-usage.patch
> 
> diff --git a/meta/recipes-support/libnl/libnl/0001-netlink.h-include-standard-poll.h-instead-of-sys-pol.patch b/meta/recipes-support/libnl/libnl/0001-netlink.h-include-standard-poll.h-instead-of-sys-pol.patch
> new file mode 100644
> index 0000000..c9096ce
> --- /dev/null
> +++ b/meta/recipes-support/libnl/libnl/0001-netlink.h-include-standard-poll.h-instead-of-sys-pol.patch
> @@ -0,0 +1,40 @@
> +From 7f73d736db539eb4c6e7d59ed9332615d10ea785 Mon Sep 17 00:00:00 2001
> +From: =?UTF-8?q?Andr=C3=A9=20Draszik?= <git@andred.net>
> +Date: Thu, 11 Aug 2016 11:53:44 +0100
> +Subject: [PATCH 1/2] netlink.h: #include standard poll.h instead of sys/poll.h
> +MIME-Version: 1.0
> +Content-Type: text/plain; charset=UTF-8
> +Content-Transfer-Encoding: 8bit
> +
> +musl issues a #warning if a non-standard file like is included
> +instead of the posix version (sys/poll.h vs poll.h in this
> +case).
> +Given netlink.h is installed and intended to be used by other
> +projects, those other projects fail to compile if they enable
> +-Werror (e.g. crda).
> +
> +Just do the correct thing and #include <poll.h> (the file
> +specified by posix).
> +
> +Signed-off-by: André Draszik <git@andred.net>
> +---
> +Upstream-Status: Pending
> + include/netlink/netlink.h | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/include/netlink/netlink.h b/include/netlink/netlink.h
> +index 28dba06..61656b3 100644
> +--- a/include/netlink/netlink.h
> ++++ b/include/netlink/netlink.h
> +@@ -16,7 +16,7 @@
> + #include <stdint.h>
> + #include <string.h>
> + #include <stdlib.h>
> +-#include <sys/poll.h>
> ++#include <poll.h>
> + #include <sys/socket.h>
> + #include <sys/types.h>
> + #include <sys/time.h>
> +--
> +2.8.1
> +
> diff --git a/meta/recipes-support/libnl/libnl/0002-musl-fix-strerror_r-usage.patch b/meta/recipes-support/libnl/libnl/0002-musl-fix-strerror_r-usage.patch
> new file mode 100644
> index 0000000..3ded5b1
> --- /dev/null
> +++ b/meta/recipes-support/libnl/libnl/0002-musl-fix-strerror_r-usage.patch
> @@ -0,0 +1,124 @@
> +From 4a88a12742ffc0a3ba3b4d4bd1398fa278ab1738 Mon Sep 17 00:00:00 2001
> +From: =?UTF-8?q?Andr=C3=A9=20Draszik?= <git@andred.net>
> +Date: Thu, 11 Aug 2016 13:15:23 +0100
> +Subject: [PATCH 2/2] musl: fix strerror_r() usage
> +MIME-Version: 1.0
> +Content-Type: text/plain; charset=UTF-8
> +Content-Transfer-Encoding: 8bit
> +
> +glibc provides two versions of strerror_r(), which
> +can be chosen between using feature test macros
> +_GNU_SOURCE and _POSIX_C_SOURCE.
> +musl on the other hand only ever provides the posix
> +version of strerror_r().
> +
> +While this patch is not switching to the XSI version
> +of strerror_r() in general, all implementations of
> +the XSI version I have checked (bionic, uclibc,
> +musl, glibc), fill buf with as many bytes as possible
> +and NUL-terminate buf correctly.
> +
> +Switch to using XSI semantics. (This patch is only
> +intended to be applied when compiling against
> +musl - unfortunately musl doesn't provide any
> +preprocessor directives to detect use of musl.)
> +
> +Signed-off-by: André Draszik <git@andred.net>
> +---
> +Upstream-Status: Inappropriate [musl specific]
> + lib/fib_lookup/lookup.c | 5 ++++-
> + lib/handlers.c          | 3 ++-
> + lib/msg.c               | 3 ++-
> + lib/route/route_obj.c   | 3 ++-
> + src/lib/utils.c         | 6 ++++--
> + 5 files changed, 14 insertions(+), 6 deletions(-)
> +
> +diff --git a/lib/fib_lookup/lookup.c b/lib/fib_lookup/lookup.c
> +index 3d07317..490f04e 100644
> +--- a/lib/fib_lookup/lookup.c
> ++++ b/lib/fib_lookup/lookup.c
> +@@ -125,6 +125,9 @@ static void result_dump_line(struct nl_object *obj, struct nl_dump_params *p)
> + {
> + 	struct flnl_result *res = (struct flnl_result *) obj;
> + 	char buf[256];
> ++	char buf2[256];
> ++
> ++	strerror_r(-res->fr_error, buf2, sizeof(buf2));
> +
> + 	nl_dump_line(p, "table %s prefixlen %u next-hop-selector %u\n",
> + 		rtnl_route_table2str(res->fr_table_id, buf, sizeof(buf)),
> +@@ -133,7 +136,7 @@ static void result_dump_line(struct nl_object *obj, struct nl_dump_params *p)
> + 		     nl_rtntype2str(res->fr_type, buf, sizeof(buf)));
> + 	nl_dump(p, "scope %s error %s (%d)\n",
> + 		rtnl_scope2str(res->fr_scope, buf, sizeof(buf)),
> +-		strerror_r(-res->fr_error, buf, sizeof(buf)), res->fr_error);
> ++		buf2, res->fr_error);
> + }
> +
> + static void result_dump_details(struct nl_object *obj, struct nl_dump_params *p)
> +diff --git a/lib/handlers.c b/lib/handlers.c
> +index a6a97bb..1b9bb4f 100644
> +--- a/lib/handlers.c
> ++++ b/lib/handlers.c
> +@@ -81,8 +81,9 @@ static int nl_error_handler_verbose(struct sockaddr_nl *who,
> + 	FILE *ofd = arg ? arg : stderr;
> + 	char buf[256];
> +
> ++	strerror_r(-e->error, buf, sizeof(buf));
> + 	fprintf(ofd, "-- Error received: %s\n-- Original message: ",
> +-		strerror_r(-e->error, buf, sizeof(buf)));
> ++		buf);
> + 	print_header_content(ofd, &e->msg);
> + 	fprintf(ofd, "\n");
> +
> +diff --git a/lib/msg.c b/lib/msg.c
> +index bcf1aa8..eec2833 100644
> +--- a/lib/msg.c
> ++++ b/lib/msg.c
> +@@ -916,8 +916,9 @@ static void dump_error_msg(struct nl_msg *msg, FILE *ofd)
> + 		char buf[256];
> + 		struct nl_msg *errmsg;
> +
> ++		strerror_r(-err->error, buf, sizeof(buf));
> + 		fprintf(ofd, "    .error = %d \"%s\"\n", err->error,
> +-			strerror_r(-err->error, buf, sizeof(buf)));
> ++			buf);
> + 		fprintf(ofd, "  [ORIGINAL MESSAGE] %zu octets\n", sizeof(*hdr));
> +
> + 		errmsg = nlmsg_inherit(&err->msg);
> +diff --git a/lib/route/route_obj.c b/lib/route/route_obj.c
> +index dd4bf49..6fc4321 100644
> +--- a/lib/route/route_obj.c
> ++++ b/lib/route/route_obj.c
> +@@ -257,9 +257,10 @@ static void route_dump_details(struct nl_object *a, struct nl_dump_params *p)
> + 	}
> +
> + 	if ((r->ce_mask & ROUTE_ATTR_CACHEINFO) && r->rt_cacheinfo.rtci_error) {
> ++		strerror_r(-r->rt_cacheinfo.rtci_error, buf, sizeof(buf));
> + 		nl_dump_line(p, "    cacheinfo error %d (%s)\n",
> + 			r->rt_cacheinfo.rtci_error,
> +-			strerror_r(-r->rt_cacheinfo.rtci_error, buf, sizeof(buf)));
> ++			buf);
> + 	}
> +
> + 	if (r->ce_mask & ROUTE_ATTR_METRICS) {
> +diff --git a/src/lib/utils.c b/src/lib/utils.c
> +index e5eacde..d9cc7a8 100644
> +--- a/src/lib/utils.c
> ++++ b/src/lib/utils.c
> +@@ -79,8 +79,10 @@ void nl_cli_fatal(int err, const char *fmt, ...)
> + 		vfprintf(stderr, fmt, ap);
> + 		va_end(ap);
> + 		fprintf(stderr, "\n");
> +-	} else
> +-		fprintf(stderr, "%s\n", strerror_r(err, buf, sizeof(buf)));
> ++	} else {
> ++		strerror_r(err, buf, sizeof(buf));
> ++		fprintf(stderr, "%s\n", buf);
> ++	}
> +
> + 	exit(abs(err));
> + }
> +--
> +2.8.1
> +
> diff --git a/meta/recipes-support/libnl/libnl_3.2.25.bb b/meta/recipes-support/libnl/libnl_3.2.25.bb
> index cabe841..540b019 100644
> --- a/meta/recipes-support/libnl/libnl_3.2.25.bb
> +++ b/meta/recipes-support/libnl/libnl_3.2.25.bb
> @@ -13,7 +13,9 @@ DEPENDS = "flex-native bison-native"
> SRC_URI = "http://www.infradead.org/~tgr/${BPN}/files/${BP}.tar.gz \
>            file://fix-pktloc_syntax_h-race.patch \
>            file://fix-pc-file.patch \
> +           file://0001-netlink.h-include-standard-poll.h-instead-of-sys-pol.patch \
>           "
> +SRC_URI_append_libc-musl = " file://0002-musl-fix-strerror_r-usage.patch"
> 
> SRC_URI[md5sum] = "03f74d0cd5037cadc8cdfa313bbd195c"
> SRC_URI[sha256sum] = "8beb7590674957b931de6b7f81c530b85dc7c1ad8fbda015398bc1e8d1ce8ec5"
> --
> 2.8.1
> 
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 211 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] libnl: musl fixes
  2016-08-12  5:45 ` Khem Raj
@ 2016-08-12  7:26   ` André Draszik
  2016-08-12  7:28     ` Khem Raj
  0 siblings, 1 reply; 10+ messages in thread
From: André Draszik @ 2016-08-12  7:26 UTC (permalink / raw)
  To: openembedded-core

On Thu, 2016-08-11 at 22:45 -0700, Khem Raj wrote:
> > 
> > On Aug 11, 2016, at 7:06 AM, André Draszik <git@andred.net> wrote:
> > 
> > The libnl+musl combination has two issues at the moment:
> > a) a public header file #include's an incorrect file
> >   (sys/poll.h instead of poll.h), which causes warnings and
> >   potentially errors (-Werror) in a musl based system
> > b) musl only ever provides the XSI version of strerror_r()
> > 
> 
> yes so you can check for something like
> (_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && ! _GNU_SOURCE
> use XSI version
> else use GNU version

Well, the feature test is supposed to be set by the user (libnl in this
case), and evaluated by the c library (musl). libnl explicitly sets
_GNU_SOURCE, i.e. it expects the glibc version.
Given musl doesn't provide the glibc version, and apart from implementing
the other version in musl, I don't see any viable alternative other than
what I posted.
One could #undefine _GNU_SOURCE and #define POSIX_C_SOURCE as appropriate
during compilation of the affected files, but that might have other side-
effects, and would be a major deviation from upstream...


Cheers,
Andre'



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] libnl: musl fixes
  2016-08-12  7:26   ` André Draszik
@ 2016-08-12  7:28     ` Khem Raj
  2016-08-12  7:42       ` André Draszik
  0 siblings, 1 reply; 10+ messages in thread
From: Khem Raj @ 2016-08-12  7:28 UTC (permalink / raw)
  To: André Draszik; +Cc: Patches and discussions about the oe-core layer

On Fri, Aug 12, 2016 at 12:26 AM, André Draszik <git@andred.net> wrote:
> On Thu, 2016-08-11 at 22:45 -0700, Khem Raj wrote:
>> >
>> > On Aug 11, 2016, at 7:06 AM, André Draszik <git@andred.net> wrote:
>> >
>> > The libnl+musl combination has two issues at the moment:
>> > a) a public header file #include's an incorrect file
>> >   (sys/poll.h instead of poll.h), which causes warnings and
>> >   potentially errors (-Werror) in a musl based system
>> > b) musl only ever provides the XSI version of strerror_r()
>> >
>>
>> yes so you can check for something like
>> (_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && ! _GNU_SOURCE
>> use XSI version
>> else use GNU version
>
> Well, the feature test is supposed to be set by the user (libnl in this
> case), and evaluated by the c library (musl). libnl explicitly sets
> _GNU_SOURCE, i.e. it expects the glibc version.

I see. It is something which should atleast be brought up with
upstream as an issue
they may just say, we need GNU version but atleast we will know

> Given musl doesn't provide the glibc version, and apart from implementing
> the other version in musl, I don't see any viable alternative other than
> what I posted.
> One could #undefine _GNU_SOURCE and #define POSIX_C_SOURCE as appropriate
> during compilation of the affected files, but that might have other side-
> effects, and would be a major deviation from upstream...


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] libnl: musl fixes
  2016-08-12  7:28     ` Khem Raj
@ 2016-08-12  7:42       ` André Draszik
  2016-08-20 14:51         ` Khem Raj
  0 siblings, 1 reply; 10+ messages in thread
From: André Draszik @ 2016-08-12  7:42 UTC (permalink / raw)
  To: Patches and discussions about the oe-core layer

On Fri, 2016-08-12 at 00:28 -0700, Khem Raj wrote:
> On Fri, Aug 12, 2016 at 12:26 AM, André Draszik <git@andred.net> wrote:
> > 
> > On Thu, 2016-08-11 at 22:45 -0700, Khem Raj wrote:
> > > 
> > > > 
> > > > 
> > > > On Aug 11, 2016, at 7:06 AM, André Draszik <git@andred.net> wrote:
> > > > 
> > > > The libnl+musl combination has two issues at the moment:
> > > > a) a public header file #include's an incorrect file
> > > >   (sys/poll.h instead of poll.h), which causes warnings and
> > > >   potentially errors (-Werror) in a musl based system
> > > > b) musl only ever provides the XSI version of strerror_r()
> > > > 
> > > 
> > > yes so you can check for something like
> > > (_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && ! _GNU_SOURCE
> > > use XSI version
> > > else use GNU version
> > 
> > Well, the feature test is supposed to be set by the user (libnl in this
> > case), and evaluated by the c library (musl). libnl explicitly sets
> > _GNU_SOURCE, i.e. it expects the glibc version.
> 
> I see. It is something which should atleast be brought up with
> upstream as an issue
> they may just say, we need GNU version but atleast we will know

It's actually quite a bad thing... There might be many users (you mentioned
perf) out there which all are silently breaking because of this missing
interface in musl. If you look at the patch, in the musl case the result was
effectively a printf of an *int* with a *string format* specifier...

Maybe it'd be worth to convince musl to implement it (uclibc does), or at
least to emit an #error?

I only noticed by coincidence...


Cheers,
Andre'



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] libnl: musl fixes
  2016-08-11 14:06 [PATCH] libnl: musl fixes André Draszik
  2016-08-12  5:45 ` Khem Raj
@ 2016-08-19 10:31 ` André Draszik
  2016-08-19 16:50   ` Khem Raj
  1 sibling, 1 reply; 10+ messages in thread
From: André Draszik @ 2016-08-19 10:31 UTC (permalink / raw)
  To: openembedded-core

ping?

A.


On Do, 2016-08-11 at 15:06 +0100, André Draszik wrote:
> The libnl+musl combination has two issues at the moment:
> a) a public header file #include's an incorrect file
>    (sys/poll.h instead of poll.h), which causes warnings and
>    potentially errors (-Werror) in a musl based system
> b) musl only ever provides the XSI version of strerror_r()
> 
> a) can be fixed using a generic patch
> b) has to be applied on demand for musl-based systems, only
> 
> Signed-off-by: André Draszik <git@andred.net>
> ---
>  ...nclude-standard-poll.h-instead-of-sys-pol.patch |  40 +++++++
>  .../libnl/0002-musl-fix-strerror_r-usage.patch     | 124
> +++++++++++++++++++++
>  meta/recipes-support/libnl/libnl_3.2.25.bb         |   2 +
>  3 files changed, 166 insertions(+)
>  create mode 100644 meta/recipes-support/libnl/libnl/0001-netlink.h-
> include-standard-poll.h-instead-of-sys-pol.patch
>  create mode 100644 meta/recipes-support/libnl/libnl/0002-musl-fix-
> strerror_r-usage.patch
> 
> diff --git a/meta/recipes-support/libnl/libnl/0001-netlink.h-include-
> standard-poll.h-instead-of-sys-pol.patch b/meta/recipes-
> support/libnl/libnl/0001-netlink.h-include-standard-poll.h-instead-of-sys-
> pol.patch
> new file mode 100644
> index 0000000..c9096ce
> --- /dev/null
> +++ b/meta/recipes-support/libnl/libnl/0001-netlink.h-include-standard-
> poll.h-instead-of-sys-pol.patch
> @@ -0,0 +1,40 @@
> +From 7f73d736db539eb4c6e7d59ed9332615d10ea785 Mon Sep 17 00:00:00 2001
> +From: =?UTF-8?q?Andr=C3=A9=20Draszik?= <git@andred.net>
> +Date: Thu, 11 Aug 2016 11:53:44 +0100
> +Subject: [PATCH 1/2] netlink.h: #include standard poll.h instead of
> sys/poll.h
> +MIME-Version: 1.0
> +Content-Type: text/plain; charset=UTF-8
> +Content-Transfer-Encoding: 8bit
> +
> +musl issues a #warning if a non-standard file like is included
> +instead of the posix version (sys/poll.h vs poll.h in this
> +case).
> +Given netlink.h is installed and intended to be used by other
> +projects, those other projects fail to compile if they enable
> +-Werror (e.g. crda).
> +
> +Just do the correct thing and #include <poll.h> (the file
> +specified by posix).
> +
> +Signed-off-by: André Draszik <git@andred.net>
> +---
> +Upstream-Status: Pending
> + include/netlink/netlink.h | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/include/netlink/netlink.h b/include/netlink/netlink.h
> +index 28dba06..61656b3 100644
> +--- a/include/netlink/netlink.h
> ++++ b/include/netlink/netlink.h
> +@@ -16,7 +16,7 @@
> + #include <stdint.h>
> + #include <string.h>
> + #include <stdlib.h>
> +-#include <sys/poll.h>
> ++#include <poll.h>
> + #include <sys/socket.h>
> + #include <sys/types.h>
> + #include <sys/time.h>
> +-- 
> +2.8.1
> +
> diff --git a/meta/recipes-support/libnl/libnl/0002-musl-fix-strerror_r-
> usage.patch b/meta/recipes-support/libnl/libnl/0002-musl-fix-strerror_r-
> usage.patch
> new file mode 100644
> index 0000000..3ded5b1
> --- /dev/null
> +++ b/meta/recipes-support/libnl/libnl/0002-musl-fix-strerror_r-
> usage.patch
> @@ -0,0 +1,124 @@
> +From 4a88a12742ffc0a3ba3b4d4bd1398fa278ab1738 Mon Sep 17 00:00:00 2001
> +From: =?UTF-8?q?Andr=C3=A9=20Draszik?= <git@andred.net>
> +Date: Thu, 11 Aug 2016 13:15:23 +0100
> +Subject: [PATCH 2/2] musl: fix strerror_r() usage
> +MIME-Version: 1.0
> +Content-Type: text/plain; charset=UTF-8
> +Content-Transfer-Encoding: 8bit
> +
> +glibc provides two versions of strerror_r(), which
> +can be chosen between using feature test macros
> +_GNU_SOURCE and _POSIX_C_SOURCE.
> +musl on the other hand only ever provides the posix
> +version of strerror_r().
> +
> +While this patch is not switching to the XSI version
> +of strerror_r() in general, all implementations of
> +the XSI version I have checked (bionic, uclibc,
> +musl, glibc), fill buf with as many bytes as possible
> +and NUL-terminate buf correctly.
> +
> +Switch to using XSI semantics. (This patch is only
> +intended to be applied when compiling against
> +musl - unfortunately musl doesn't provide any
> +preprocessor directives to detect use of musl.)
> +
> +Signed-off-by: André Draszik <git@andred.net>
> +---
> +Upstream-Status: Inappropriate [musl specific]
> + lib/fib_lookup/lookup.c | 5 ++++-
> + lib/handlers.c          | 3 ++-
> + lib/msg.c               | 3 ++-
> + lib/route/route_obj.c   | 3 ++-
> + src/lib/utils.c         | 6 ++++--
> + 5 files changed, 14 insertions(+), 6 deletions(-)
> +
> +diff --git a/lib/fib_lookup/lookup.c b/lib/fib_lookup/lookup.c
> +index 3d07317..490f04e 100644
> +--- a/lib/fib_lookup/lookup.c
> ++++ b/lib/fib_lookup/lookup.c
> +@@ -125,6 +125,9 @@ static void result_dump_line(struct nl_object *obj,
> struct nl_dump_params *p)
> + {
> + 	struct flnl_result *res = (struct flnl_result *) obj;
> + 	char buf[256];
> ++	char buf2[256];
> ++
> ++	strerror_r(-res->fr_error, buf2, sizeof(buf2));
> + 
> + 	nl_dump_line(p, "table %s prefixlen %u next-hop-selector %u\n",
> + 		rtnl_route_table2str(res->fr_table_id, buf,
> sizeof(buf)),
> +@@ -133,7 +136,7 @@ static void result_dump_line(struct nl_object *obj,
> struct nl_dump_params *p)
> + 		     nl_rtntype2str(res->fr_type, buf, sizeof(buf)));
> + 	nl_dump(p, "scope %s error %s (%d)\n",
> + 		rtnl_scope2str(res->fr_scope, buf, sizeof(buf)),
> +-		strerror_r(-res->fr_error, buf, sizeof(buf)), res-
> >fr_error);
> ++		buf2, res->fr_error);
> + }
> + 
> + static void result_dump_details(struct nl_object *obj, struct
> nl_dump_params *p)
> +diff --git a/lib/handlers.c b/lib/handlers.c
> +index a6a97bb..1b9bb4f 100644
> +--- a/lib/handlers.c
> ++++ b/lib/handlers.c
> +@@ -81,8 +81,9 @@ static int nl_error_handler_verbose(struct sockaddr_nl
> *who,
> + 	FILE *ofd = arg ? arg : stderr;
> + 	char buf[256];
> + 
> ++	strerror_r(-e->error, buf, sizeof(buf));
> + 	fprintf(ofd, "-- Error received: %s\n-- Original message: ",
> +-		strerror_r(-e->error, buf, sizeof(buf)));
> ++		buf);
> + 	print_header_content(ofd, &e->msg);
> + 	fprintf(ofd, "\n");
> + 
> +diff --git a/lib/msg.c b/lib/msg.c
> +index bcf1aa8..eec2833 100644
> +--- a/lib/msg.c
> ++++ b/lib/msg.c
> +@@ -916,8 +916,9 @@ static void dump_error_msg(struct nl_msg *msg, FILE
> *ofd)
> + 		char buf[256];
> + 		struct nl_msg *errmsg;
> + 
> ++		strerror_r(-err->error, buf, sizeof(buf));
> + 		fprintf(ofd, "    .error = %d \"%s\"\n", err->error,
> +-			strerror_r(-err->error, buf, sizeof(buf)));
> ++			buf);
> + 		fprintf(ofd, "  [ORIGINAL MESSAGE] %zu octets\n",
> sizeof(*hdr));
> + 
> + 		errmsg = nlmsg_inherit(&err->msg);
> +diff --git a/lib/route/route_obj.c b/lib/route/route_obj.c
> +index dd4bf49..6fc4321 100644
> +--- a/lib/route/route_obj.c
> ++++ b/lib/route/route_obj.c
> +@@ -257,9 +257,10 @@ static void route_dump_details(struct nl_object *a,
> struct nl_dump_params *p)
> + 	}
> + 
> + 	if ((r->ce_mask & ROUTE_ATTR_CACHEINFO) && r-
> >rt_cacheinfo.rtci_error) {
> ++		strerror_r(-r->rt_cacheinfo.rtci_error, buf,
> sizeof(buf));
> + 		nl_dump_line(p, "    cacheinfo error %d (%s)\n",
> + 			r->rt_cacheinfo.rtci_error,
> +-			strerror_r(-r->rt_cacheinfo.rtci_error, buf,
> sizeof(buf)));
> ++			buf);
> + 	}
> + 
> + 	if (r->ce_mask & ROUTE_ATTR_METRICS) {
> +diff --git a/src/lib/utils.c b/src/lib/utils.c
> +index e5eacde..d9cc7a8 100644
> +--- a/src/lib/utils.c
> ++++ b/src/lib/utils.c
> +@@ -79,8 +79,10 @@ void nl_cli_fatal(int err, const char *fmt, ...)
> + 		vfprintf(stderr, fmt, ap);
> + 		va_end(ap);
> + 		fprintf(stderr, "\n");
> +-	} else
> +-		fprintf(stderr, "%s\n", strerror_r(err, buf,
> sizeof(buf)));
> ++	} else {
> ++		strerror_r(err, buf, sizeof(buf));
> ++		fprintf(stderr, "%s\n", buf);
> ++	}
> + 
> + 	exit(abs(err));
> + }
> +-- 
> +2.8.1
> +
> diff --git a/meta/recipes-support/libnl/libnl_3.2.25.bb b/meta/recipes-
> support/libnl/libnl_3.2.25.bb
> index cabe841..540b019 100644
> --- a/meta/recipes-support/libnl/libnl_3.2.25.bb
> +++ b/meta/recipes-support/libnl/libnl_3.2.25.bb
> @@ -13,7 +13,9 @@ DEPENDS = "flex-native bison-native"
>  SRC_URI = "http://www.infradead.org/~tgr/${BPN}/files/${BP}.tar.gz \
>             file://fix-pktloc_syntax_h-race.patch \
>             file://fix-pc-file.patch \
> +           file://0001-netlink.h-include-standard-poll.h-instead-of-sys-
> pol.patch \
>            "
> +SRC_URI_append_libc-musl = " file://0002-musl-fix-strerror_r-usage.patch"
>  
>  SRC_URI[md5sum] = "03f74d0cd5037cadc8cdfa313bbd195c"
>  SRC_URI[sha256sum] =
> "8beb7590674957b931de6b7f81c530b85dc7c1ad8fbda015398bc1e8d1ce8ec5"


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] libnl: musl fixes
  2016-08-19 10:31 ` André Draszik
@ 2016-08-19 16:50   ` Khem Raj
  0 siblings, 0 replies; 10+ messages in thread
From: Khem Raj @ 2016-08-19 16:50 UTC (permalink / raw)
  To: André Draszik; +Cc: openembedded-core

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

OK this can be merged.

> On Aug 19, 2016, at 3:31 AM, André Draszik <git@andred.net> wrote:
> 
> ping?
> 
> A.
> 
> 
> On Do, 2016-08-11 at 15:06 +0100, André Draszik wrote:
>> The libnl+musl combination has two issues at the moment:
>> a) a public header file #include's an incorrect file
>>    (sys/poll.h instead of poll.h), which causes warnings and
>>    potentially errors (-Werror) in a musl based system
>> b) musl only ever provides the XSI version of strerror_r()
>> 
>> a) can be fixed using a generic patch
>> b) has to be applied on demand for musl-based systems, only
>> 
>> Signed-off-by: André Draszik <git@andred.net>
>> ---
>>  ...nclude-standard-poll.h-instead-of-sys-pol.patch |  40 +++++++
>>  .../libnl/0002-musl-fix-strerror_r-usage.patch     | 124
>> +++++++++++++++++++++
>>  meta/recipes-support/libnl/libnl_3.2.25.bb         |   2 +
>>  3 files changed, 166 insertions(+)
>>  create mode 100644 meta/recipes-support/libnl/libnl/0001-netlink.h-
>> include-standard-poll.h-instead-of-sys-pol.patch
>>  create mode 100644 meta/recipes-support/libnl/libnl/0002-musl-fix-
>> strerror_r-usage.patch
>> 
>> diff --git a/meta/recipes-support/libnl/libnl/0001-netlink.h-include-
>> standard-poll.h-instead-of-sys-pol.patch b/meta/recipes-
>> support/libnl/libnl/0001-netlink.h-include-standard-poll.h-instead-of-sys-
>> pol.patch
>> new file mode 100644
>> index 0000000..c9096ce
>> --- /dev/null
>> +++ b/meta/recipes-support/libnl/libnl/0001-netlink.h-include-standard-
>> poll.h-instead-of-sys-pol.patch
>> @@ -0,0 +1,40 @@
>> +From 7f73d736db539eb4c6e7d59ed9332615d10ea785 Mon Sep 17 00:00:00 2001
>> +From: =?UTF-8?q?Andr=C3=A9=20Draszik?= <git@andred.net>
>> +Date: Thu, 11 Aug 2016 11:53:44 +0100
>> +Subject: [PATCH 1/2] netlink.h: #include standard poll.h instead of
>> sys/poll.h
>> +MIME-Version: 1.0
>> +Content-Type: text/plain; charset=UTF-8
>> +Content-Transfer-Encoding: 8bit
>> +
>> +musl issues a #warning if a non-standard file like is included
>> +instead of the posix version (sys/poll.h vs poll.h in this
>> +case).
>> +Given netlink.h is installed and intended to be used by other
>> +projects, those other projects fail to compile if they enable
>> +-Werror (e.g. crda).
>> +
>> +Just do the correct thing and #include <poll.h> (the file
>> +specified by posix).
>> +
>> +Signed-off-by: André Draszik <git@andred.net>
>> +---
>> +Upstream-Status: Pending
>> + include/netlink/netlink.h | 2 +-
>> + 1 file changed, 1 insertion(+), 1 deletion(-)
>> +
>> +diff --git a/include/netlink/netlink.h b/include/netlink/netlink.h
>> +index 28dba06..61656b3 100644
>> +--- a/include/netlink/netlink.h
>> ++++ b/include/netlink/netlink.h
>> +@@ -16,7 +16,7 @@
>> + #include <stdint.h>
>> + #include <string.h>
>> + #include <stdlib.h>
>> +-#include <sys/poll.h>
>> ++#include <poll.h>
>> + #include <sys/socket.h>
>> + #include <sys/types.h>
>> + #include <sys/time.h>
>> +--
>> +2.8.1
>> +
>> diff --git a/meta/recipes-support/libnl/libnl/0002-musl-fix-strerror_r-
>> usage.patch b/meta/recipes-support/libnl/libnl/0002-musl-fix-strerror_r-
>> usage.patch
>> new file mode 100644
>> index 0000000..3ded5b1
>> --- /dev/null
>> +++ b/meta/recipes-support/libnl/libnl/0002-musl-fix-strerror_r-
>> usage.patch
>> @@ -0,0 +1,124 @@
>> +From 4a88a12742ffc0a3ba3b4d4bd1398fa278ab1738 Mon Sep 17 00:00:00 2001
>> +From: =?UTF-8?q?Andr=C3=A9=20Draszik?= <git@andred.net>
>> +Date: Thu, 11 Aug 2016 13:15:23 +0100
>> +Subject: [PATCH 2/2] musl: fix strerror_r() usage
>> +MIME-Version: 1.0
>> +Content-Type: text/plain; charset=UTF-8
>> +Content-Transfer-Encoding: 8bit
>> +
>> +glibc provides two versions of strerror_r(), which
>> +can be chosen between using feature test macros
>> +_GNU_SOURCE and _POSIX_C_SOURCE.
>> +musl on the other hand only ever provides the posix
>> +version of strerror_r().
>> +
>> +While this patch is not switching to the XSI version
>> +of strerror_r() in general, all implementations of
>> +the XSI version I have checked (bionic, uclibc,
>> +musl, glibc), fill buf with as many bytes as possible
>> +and NUL-terminate buf correctly.
>> +
>> +Switch to using XSI semantics. (This patch is only
>> +intended to be applied when compiling against
>> +musl - unfortunately musl doesn't provide any
>> +preprocessor directives to detect use of musl.)
>> +
>> +Signed-off-by: André Draszik <git@andred.net>
>> +---
>> +Upstream-Status: Inappropriate [musl specific]
>> + lib/fib_lookup/lookup.c | 5 ++++-
>> + lib/handlers.c          | 3 ++-
>> + lib/msg.c               | 3 ++-
>> + lib/route/route_obj.c   | 3 ++-
>> + src/lib/utils.c         | 6 ++++--
>> + 5 files changed, 14 insertions(+), 6 deletions(-)
>> +
>> +diff --git a/lib/fib_lookup/lookup.c b/lib/fib_lookup/lookup.c
>> +index 3d07317..490f04e 100644
>> +--- a/lib/fib_lookup/lookup.c
>> ++++ b/lib/fib_lookup/lookup.c
>> +@@ -125,6 +125,9 @@ static void result_dump_line(struct nl_object *obj,
>> struct nl_dump_params *p)
>> + {
>> + 	struct flnl_result *res = (struct flnl_result *) obj;
>> + 	char buf[256];
>> ++	char buf2[256];
>> ++
>> ++	strerror_r(-res->fr_error, buf2, sizeof(buf2));
>> +
>> + 	nl_dump_line(p, "table %s prefixlen %u next-hop-selector %u\n",
>> + 		rtnl_route_table2str(res->fr_table_id, buf,
>> sizeof(buf)),
>> +@@ -133,7 +136,7 @@ static void result_dump_line(struct nl_object *obj,
>> struct nl_dump_params *p)
>> + 		     nl_rtntype2str(res->fr_type, buf, sizeof(buf)));
>> + 	nl_dump(p, "scope %s error %s (%d)\n",
>> + 		rtnl_scope2str(res->fr_scope, buf, sizeof(buf)),
>> +-		strerror_r(-res->fr_error, buf, sizeof(buf)), res-
>>> fr_error);
>> ++		buf2, res->fr_error);
>> + }
>> +
>> + static void result_dump_details(struct nl_object *obj, struct
>> nl_dump_params *p)
>> +diff --git a/lib/handlers.c b/lib/handlers.c
>> +index a6a97bb..1b9bb4f 100644
>> +--- a/lib/handlers.c
>> ++++ b/lib/handlers.c
>> +@@ -81,8 +81,9 @@ static int nl_error_handler_verbose(struct sockaddr_nl
>> *who,
>> + 	FILE *ofd = arg ? arg : stderr;
>> + 	char buf[256];
>> +
>> ++	strerror_r(-e->error, buf, sizeof(buf));
>> + 	fprintf(ofd, "-- Error received: %s\n-- Original message: ",
>> +-		strerror_r(-e->error, buf, sizeof(buf)));
>> ++		buf);
>> + 	print_header_content(ofd, &e->msg);
>> + 	fprintf(ofd, "\n");
>> +
>> +diff --git a/lib/msg.c b/lib/msg.c
>> +index bcf1aa8..eec2833 100644
>> +--- a/lib/msg.c
>> ++++ b/lib/msg.c
>> +@@ -916,8 +916,9 @@ static void dump_error_msg(struct nl_msg *msg, FILE
>> *ofd)
>> + 		char buf[256];
>> + 		struct nl_msg *errmsg;
>> +
>> ++		strerror_r(-err->error, buf, sizeof(buf));
>> + 		fprintf(ofd, "    .error = %d \"%s\"\n", err->error,
>> +-			strerror_r(-err->error, buf, sizeof(buf)));
>> ++			buf);
>> + 		fprintf(ofd, "  [ORIGINAL MESSAGE] %zu octets\n",
>> sizeof(*hdr));
>> +
>> + 		errmsg = nlmsg_inherit(&err->msg);
>> +diff --git a/lib/route/route_obj.c b/lib/route/route_obj.c
>> +index dd4bf49..6fc4321 100644
>> +--- a/lib/route/route_obj.c
>> ++++ b/lib/route/route_obj.c
>> +@@ -257,9 +257,10 @@ static void route_dump_details(struct nl_object *a,
>> struct nl_dump_params *p)
>> + 	}
>> +
>> + 	if ((r->ce_mask & ROUTE_ATTR_CACHEINFO) && r-
>>> rt_cacheinfo.rtci_error) {
>> ++		strerror_r(-r->rt_cacheinfo.rtci_error, buf,
>> sizeof(buf));
>> + 		nl_dump_line(p, "    cacheinfo error %d (%s)\n",
>> + 			r->rt_cacheinfo.rtci_error,
>> +-			strerror_r(-r->rt_cacheinfo.rtci_error, buf,
>> sizeof(buf)));
>> ++			buf);
>> + 	}
>> +
>> + 	if (r->ce_mask & ROUTE_ATTR_METRICS) {
>> +diff --git a/src/lib/utils.c b/src/lib/utils.c
>> +index e5eacde..d9cc7a8 100644
>> +--- a/src/lib/utils.c
>> ++++ b/src/lib/utils.c
>> +@@ -79,8 +79,10 @@ void nl_cli_fatal(int err, const char *fmt, ...)
>> + 		vfprintf(stderr, fmt, ap);
>> + 		va_end(ap);
>> + 		fprintf(stderr, "\n");
>> +-	} else
>> +-		fprintf(stderr, "%s\n", strerror_r(err, buf,
>> sizeof(buf)));
>> ++	} else {
>> ++		strerror_r(err, buf, sizeof(buf));
>> ++		fprintf(stderr, "%s\n", buf);
>> ++	}
>> +
>> + 	exit(abs(err));
>> + }
>> +--
>> +2.8.1
>> +
>> diff --git a/meta/recipes-support/libnl/libnl_3.2.25.bb b/meta/recipes-
>> support/libnl/libnl_3.2.25.bb
>> index cabe841..540b019 100644
>> --- a/meta/recipes-support/libnl/libnl_3.2.25.bb
>> +++ b/meta/recipes-support/libnl/libnl_3.2.25.bb
>> @@ -13,7 +13,9 @@ DEPENDS = "flex-native bison-native"
>>  SRC_URI = "http://www.infradead.org/~tgr/${BPN}/files/${BP}.tar.gz \
>>             file://fix-pktloc_syntax_h-race.patch \
>>             file://fix-pc-file.patch \
>> +           file://0001-netlink.h-include-standard-poll.h-instead-of-sys-
>> pol.patch \
>>            "
>> +SRC_URI_append_libc-musl = " file://0002-musl-fix-strerror_r-usage.patch"
>> 
>>  SRC_URI[md5sum] = "03f74d0cd5037cadc8cdfa313bbd195c"
>>  SRC_URI[sha256sum] =
>> "8beb7590674957b931de6b7f81c530b85dc7c1ad8fbda015398bc1e8d1ce8ec5"
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 211 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] libnl: musl fixes
  2016-08-12  7:42       ` André Draszik
@ 2016-08-20 14:51         ` Khem Raj
  2016-08-22  9:54           ` André Draszik
  2016-08-26 10:42           ` André Draszik
  0 siblings, 2 replies; 10+ messages in thread
From: Khem Raj @ 2016-08-20 14:51 UTC (permalink / raw)
  To: André Draszik; +Cc: Patches and discussions about the oe-core layer

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


> On Aug 12, 2016, at 12:42 AM, André Draszik <git@andred.net> wrote:
> 
> On Fri, 2016-08-12 at 00:28 -0700, Khem Raj wrote:
>> On Fri, Aug 12, 2016 at 12:26 AM, André Draszik <git@andred.net> wrote:
>>> 
>>> On Thu, 2016-08-11 at 22:45 -0700, Khem Raj wrote:
>>>> 
>>>>> 
>>>>> 
>>>>> On Aug 11, 2016, at 7:06 AM, André Draszik <git@andred.net> wrote:
>>>>> 
>>>>> The libnl+musl combination has two issues at the moment:
>>>>> a) a public header file #include's an incorrect file
>>>>>   (sys/poll.h instead of poll.h), which causes warnings and
>>>>>   potentially errors (-Werror) in a musl based system
>>>>> b) musl only ever provides the XSI version of strerror_r()
>>>>> 
>>>> 
>>>> yes so you can check for something like
>>>> (_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && ! _GNU_SOURCE
>>>> use XSI version
>>>> else use GNU version
>>> 
>>> Well, the feature test is supposed to be set by the user (libnl in this
>>> case), and evaluated by the c library (musl). libnl explicitly sets
>>> _GNU_SOURCE, i.e. it expects the glibc version.
>> 
>> I see. It is something which should atleast be brought up with
>> upstream as an issue
>> they may just say, we need GNU version but atleast we will know
> 
> It's actually quite a bad thing... There might be many users (you mentioned
> perf) out there which all are silently breaking because of this missing
> interface in musl. If you look at the patch, in the musl case the result was
> effectively a printf of an *int* with a *string format* specifier...
> 
> Maybe it'd be worth to convince musl to implement it (uclibc does), or at
> least to emit an #error?

Thinking more on this, there is an alternative to use strerror_l() rather
than strerror_r(), that will avoid the non-confirming versions issue, moreover
strerror_() is now recommended by posix to replace strerror_r() usage.

see

http://austingroupbugs.net/view.php?id=655

I think this is going to be a better solution since its portable.

> 
> I only noticed by coincidence...
> 
> 
> Cheers,
> Andre'
> 
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 211 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] libnl: musl fixes
  2016-08-20 14:51         ` Khem Raj
@ 2016-08-22  9:54           ` André Draszik
  2016-08-26 10:42           ` André Draszik
  1 sibling, 0 replies; 10+ messages in thread
From: André Draszik @ 2016-08-22  9:54 UTC (permalink / raw)
  To: Patches and discussions about the oe-core layer

On Sa, 2016-08-20 at 07:51 -0700, Khem Raj wrote:
> > 
> > 
> Thinking more on this, there is an alternative to use strerror_l() rather
> than strerror_r(), that will avoid the non-confirming versions issue,
> moreover
> strerror_() is now recommended by posix to replace strerror_r() usage.
> 
> see
> 
> http://austingroupbugs.net/view.php?id=655
> 
> I think this is going to be a better solution since its portable.

Yeah, you're right. Let me cook up something...


Cheers,
Andre'



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] libnl: musl fixes
  2016-08-20 14:51         ` Khem Raj
  2016-08-22  9:54           ` André Draszik
@ 2016-08-26 10:42           ` André Draszik
  1 sibling, 0 replies; 10+ messages in thread
From: André Draszik @ 2016-08-26 10:42 UTC (permalink / raw)
  To: Patches and discussions about the oe-core layer

On Sa, 2016-08-20 at 07:51 -0700, Khem Raj wrote:
> Thinking more on this, there is an alternative to use strerror_l() rather
> than strerror_r(), that will avoid the non-confirming versions issue,
> moreover
> strerror_() is now recommended by posix to replace strerror_r() usage.
> 
> see
> 
> http://austingroupbugs.net/view.php?id=655
> 
> I think this is going to be a better solution since its portable.

done:
http://lists.openembedded.org/pipermail/openembedded-core/2016-August/125733
.html



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-08-26 10:42 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-11 14:06 [PATCH] libnl: musl fixes André Draszik
2016-08-12  5:45 ` Khem Raj
2016-08-12  7:26   ` André Draszik
2016-08-12  7:28     ` Khem Raj
2016-08-12  7:42       ` André Draszik
2016-08-20 14:51         ` Khem Raj
2016-08-22  9:54           ` André Draszik
2016-08-26 10:42           ` André Draszik
2016-08-19 10:31 ` André Draszik
2016-08-19 16:50   ` Khem Raj

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.