[PATCH 1/3] remove non-portable usage of strndupa

[PATCH 1/3] remove non-portable usage of strndupa

[John Spencer]

usage of strndupa is neither C99 nor POSIX,
so musl libc does not implement it.
it's a glibc invention, and a dangerous one since
usage of alloca() is considered bad practice.

fixes build with musl libc.

Signed-off-by: John Spencer <maillist-kmod@barfooze.de>

---
 libkmod/libkmod-util.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/libkmod/libkmod-util.c b/libkmod/libkmod-util.c
index d686250..9615d0f 100644
--- a/libkmod/libkmod-util.c
+++ b/libkmod/libkmod-util.c
@@ -28,6 +28,7 @@
 #include <unistd.h>
 #include <errno.h>
 #include <string.h>
+#include <limits.h>
 #include <ctype.h>
 
 #include "libkmod.h"
@@ -323,8 +324,11 @@ static inline int is_dir(const char *path)
 int mkdir_p(const char *path, int len, mode_t mode)
 {
 	char *start, *end;
-
-	start = strndupa(path, len);
+	char buf[PATH_MAX+1];
+	snprintf(buf, sizeof buf, "%s", path);
+	assert(len < sizeof(buf));
+	buf[len] = 0;
+	start = buf;
 	end = start + len;
 
 	/*
-- 
1.7.3.4

[PATCH 2/3] check if __GLIBC__ is defined before using _FILE_OFFSET_BITS

[John Spencer]

_FILE_OFFSET_BITS is a glibc internal macro.
musl provides 64bit off_t by default, but
defines stat64 etc as macros to make glibc-centric
programs happy. however the expansion causes
problems with the hack used here to work around
glibc's 32bit past...

Signed-off-by: John Spencer <maillist-kmod@barfooze.de>

---
 testsuite/path.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/testsuite/path.c b/testsuite/path.c
index f87f5c5..f06fadd 100644
--- a/testsuite/path.c
+++ b/testsuite/path.c
@@ -185,7 +185,7 @@ WRAP_2ARGS(int, -1, mkdir, mode_t);
 WRAP_2ARGS(int, -1, access, int);
 WRAP_2ARGS(int, -1, stat, struct stat*);
 WRAP_2ARGS(int, -1, lstat, struct stat*);
-#ifndef _FILE_OFFSET_BITS
+#if defined(__GLIBC__) && !defined(_FILE_OFFSET_BITS)
 WRAP_2ARGS(int, -1, stat64, struct stat64*);
 WRAP_2ARGS(int, -1, lstat64, struct stat64*);
 WRAP_OPEN(64);
@@ -196,7 +196,7 @@ WRAP_OPEN();
 #ifdef HAVE___XSTAT
 WRAP_VERSTAT(__x,);
 WRAP_VERSTAT(__lx,);
-#ifndef _FILE_OFFSET_BITS
+#if defined(__GLIBC__) && !defined(_FILE_OFFSET_BITS)
 WRAP_VERSTAT(__x,64);
 WRAP_VERSTAT(__lx,64);
 #endif
-- 
1.7.3.4

Re: [PATCH 2/3] check if __GLIBC__ is defined before using _FILE_OFFSET_BITS

[Lucas De Marchi]

On Mon, Aug 26, 2013 at 3:20 PM, John Spencer <maillist-kmod@barfooze.de> wrote:
> _FILE_OFFSET_BITS is a glibc internal macro.
> musl provides 64bit off_t by default, but
> defines stat64 etc as macros to make glibc-centric
> programs happy. however the expansion causes
> problems with the hack used here to work around
> glibc's 32bit past...

AFAIK this define comes from AC_SYS_LARGEFILE in autoconf and ends up
in our config.h. Checking for the libc in source code is not much
future proof and I'd like to avoid it.  If this is not the right
define to pick, is there any other one?

Maybe it would be better to do like this:

#ifndef _FILE_OFFSET_BITS
#ifndef stat64
WRAP_2ARGS(int, -1, stat64, struct stat64*);
#endif
...

Because if it's a macro, there's no need to wrap the call.

Lucas De Marchi

Re: [PATCH 2/3] check if __GLIBC__ is defined before using _FILE_OFFSET_BITS

[John Spencer]

On 08/27/2013 01:18 AM, Lucas De Marchi wrote:
> On Mon, Aug 26, 2013 at 3:20 PM, John Spencer<maillist-kmod@barfooze.de>  wrote:
>> _FILE_OFFSET_BITS is a glibc internal macro.
>> musl provides 64bit off_t by default, but
>> defines stat64 etc as macros to make glibc-centric
>> programs happy. however the expansion causes
>> problems with the hack used here to work around
>> glibc's 32bit past...
>
> AFAIK this define comes from AC_SYS_LARGEFILE in autoconf and ends up
> in our config.h. Checking for the libc in source code is not much
> future proof and I'd like to avoid it.  If this is not the right
> define to pick, is there any other one?
>
> Maybe it would be better to do like this:
>
> #ifndef _FILE_OFFSET_BITS
> #ifndef stat64
> WRAP_2ARGS(int, -1, stat64, struct stat64*);
> #endif
> ...
>
> Because if it's a macro, there's no need to wrap the call.

Yes, that would work as well.

>
> Lucas De Marchi
>

Thanks,
--JS

[PATCH 3/3] testsuite: fix usage of reserved names

[John Spencer]

stdout and stderr are names reserved for the implementation
and musl uses them rightfully as macro - and the expansion
causes (of course) unexpected results.

renaming the struct members stdout to std_out and stderr
to std_err, to be 1) compliant 2) cause compilation to
succeed.

fixes build with musl libc.

Signed-off-by: John Spencer <maillist-kmod@barfooze.de>

---
 testsuite/test-alias.c      |    2 +-
 testsuite/test-loaded.c     |    2 +-
 testsuite/test-modinfo.c    |    2 +-
 testsuite/test-modprobe.c   |    8 ++++----
 testsuite/test-new-module.c |    4 ++--
 testsuite/testsuite.c       |   30 +++++++++++++++---------------
 testsuite/testsuite.h       |    4 ++--
 7 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/testsuite/test-alias.c b/testsuite/test-alias.c
index 5e21573..a4b4c14 100644
--- a/testsuite/test-alias.c
+++ b/testsuite/test-alias.c
@@ -64,7 +64,7 @@ static DEFINE_TEST(alias_1,
 	},
 	.need_spawn = true,
 	.output = {
-		.stdout = TESTSUITE_ROOTFS "test-alias/correct.txt",
+		.std_out = TESTSUITE_ROOTFS "test-alias/correct.txt",
 	});
 
 static const struct test *tests[] = {
diff --git a/testsuite/test-loaded.c b/testsuite/test-loaded.c
index 33865cb..734a0a8 100644
--- a/testsuite/test-loaded.c
+++ b/testsuite/test-loaded.c
@@ -85,7 +85,7 @@ static DEFINE_TEST(loaded_1,
 	},
 	.need_spawn = true,
 	.output = {
-		.stdout = TESTSUITE_ROOTFS "test-loaded/correct.txt",
+		.std_out = TESTSUITE_ROOTFS "test-loaded/correct.txt",
 	});
 
 static const struct test *tests[] = {
diff --git a/testsuite/test-modinfo.c b/testsuite/test-modinfo.c
index c5934ea..c387633 100644
--- a/testsuite/test-modinfo.c
+++ b/testsuite/test-modinfo.c
@@ -46,7 +46,7 @@ static DEFINE_TEST(modinfo_jonsmodules,
 		[TC_ROOTFS] = TESTSUITE_ROOTFS "test-modinfo/",
 	},
 	.output = {
-		.stdout = TESTSUITE_ROOTFS "test-modinfo/correct.txt",
+		.std_out = TESTSUITE_ROOTFS "test-modinfo/correct.txt",
 	});
 
 static const struct test *tests[] = {
diff --git a/testsuite/test-modprobe.c b/testsuite/test-modprobe.c
index f0131e3..5dfbe22 100644
--- a/testsuite/test-modprobe.c
+++ b/testsuite/test-modprobe.c
@@ -45,7 +45,7 @@ static DEFINE_TEST(modprobe_show_depends,
 		[TC_ROOTFS] = TESTSUITE_ROOTFS "test-modprobe/show-depends",
 	},
 	.output = {
-		.stdout = TESTSUITE_ROOTFS "test-modprobe/show-depends/correct.txt",
+		.std_out = TESTSUITE_ROOTFS "test-modprobe/show-depends/correct.txt",
 	});
 
 static __noreturn int modprobe_show_depends2(const struct test *t)
@@ -67,7 +67,7 @@ static DEFINE_TEST(modprobe_show_depends2,
 		[TC_ROOTFS] = TESTSUITE_ROOTFS "test-modprobe/show-depends",
 	},
 	.output = {
-		.stdout = TESTSUITE_ROOTFS "test-modprobe/show-depends/correct-psmouse.txt",
+		.std_out = TESTSUITE_ROOTFS "test-modprobe/show-depends/correct-psmouse.txt",
 	});
 
 
@@ -90,7 +90,7 @@ static DEFINE_TEST(modprobe_show_alias_to_none,
 		[TC_ROOTFS] = TESTSUITE_ROOTFS "test-modprobe/alias-to-none",
 	},
 	.output = {
-		.stdout = TESTSUITE_ROOTFS "test-modprobe/show-depends/correct-psmouse.txt",
+		.std_out = TESTSUITE_ROOTFS "test-modprobe/show-depends/correct-psmouse.txt",
 	});
 
 
@@ -177,7 +177,7 @@ static DEFINE_TEST(modprobe_param_kcmdline,
 		[TC_ROOTFS] = TESTSUITE_ROOTFS "test-modprobe/module-param-kcmdline",
 	},
 	.output = {
-		.stdout = TESTSUITE_ROOTFS "test-modprobe/module-param-kcmdline/correct.txt",
+		.std_out = TESTSUITE_ROOTFS "test-modprobe/module-param-kcmdline/correct.txt",
 	});
 
 
diff --git a/testsuite/test-new-module.c b/testsuite/test-new-module.c
index 240480e..cb603d4 100644
--- a/testsuite/test-new-module.c
+++ b/testsuite/test-new-module.c
@@ -67,7 +67,7 @@ static DEFINE_TEST(from_name,
 	},
 	.need_spawn = true,
 	.output = {
-		.stdout = TESTSUITE_ROOTFS "test-new-module/from_name/correct.txt",
+		.std_out = TESTSUITE_ROOTFS "test-new-module/from_name/correct.txt",
 	});
 
 static int from_alias(const struct test *t)
@@ -112,7 +112,7 @@ static DEFINE_TEST(from_alias,
 	},
 	.need_spawn = true,
 	.output = {
-		.stdout = TESTSUITE_ROOTFS "test-new-module/from_alias/correct.txt",
+		.std_out = TESTSUITE_ROOTFS "test-new-module/from_alias/correct.txt",
 	});
 
 static const struct test *tests[] = {
diff --git a/testsuite/testsuite.c b/testsuite/testsuite.c
index 6d85d7f..0b66cb9 100644
--- a/testsuite/testsuite.c
+++ b/testsuite/testsuite.c
@@ -223,7 +223,7 @@ static inline int test_run_child(const struct test *t, int fdout[2],
 	test_export_environ(t);
 
 	/* Close read-fds and redirect std{out,err} to the write-fds */
-	if (t->output.stdout != NULL) {
+	if (t->output.std_out != NULL) {
 		close(fdout[0]);
 		if (dup2(fdout[1], STDOUT_FILENO) < 0) {
 			ERR("could not redirect stdout to pipe: %m\n");
@@ -231,10 +231,10 @@ static inline int test_run_child(const struct test *t, int fdout[2],
 		}
 	}
 
-	if (t->output.stderr != NULL) {
+	if (t->output.std_err != NULL) {
 		close(fderr[0]);
 		if (dup2(fderr[1], STDERR_FILENO) < 0) {
-			ERR("could not redirect stdout to pipe: %m\n");
+			ERR("could not redirect stderr to pipe: %m\n");
 			exit(EXIT_FAILURE);
 		}
 	}
@@ -282,12 +282,12 @@ static inline bool test_run_parent_check_outputs(const struct test *t,
 		return false;
 	}
 
-	if (t->output.stdout != NULL) {
-		fd_matchout = open(t->output.stdout, O_RDONLY);
+	if (t->output.std_out != NULL) {
+		fd_matchout = open(t->output.std_out, O_RDONLY);
 		if (fd_matchout < 0) {
 			err = -errno;
 			ERR("could not open %s for read: %m\n",
-							t->output.stdout);
+							t->output.std_out);
 			goto out;
 		}
 		memset(&ep_outpipe, 0, sizeof(struct epoll_event));
@@ -301,12 +301,12 @@ static inline bool test_run_parent_check_outputs(const struct test *t,
 	} else
 		fdout = -1;
 
-	if (t->output.stderr != NULL) {
-		fd_matcherr = open(t->output.stderr, O_RDONLY);
+	if (t->output.std_err != NULL) {
+		fd_matcherr = open(t->output.std_err, O_RDONLY);
 		if (fd_matcherr < 0) {
 			err = -errno;
 			ERR("could not open %s for read: %m\n",
-					t->output.stderr);
+					t->output.std_err);
 			goto out;
 
 		}
@@ -536,9 +536,9 @@ static inline int test_run_parent(const struct test *t, int fdout[2],
 	bool matchout;
 
 	/* Close write-fds */
-	if (t->output.stdout != NULL)
+	if (t->output.std_out != NULL)
 		close(fdout[1]);
-	if (t->output.stderr != NULL)
+	if (t->output.std_err != NULL)
 		close(fderr[1]);
 	close(fdmonitor[1]);
 
@@ -549,9 +549,9 @@ static inline int test_run_parent(const struct test *t, int fdout[2],
 	 * break pipe on the other end: either child already closed or we want
 	 * to stop it
 	 */
-	if (t->output.stdout != NULL)
+	if (t->output.std_out != NULL)
 		close(fdout[0]);
-	if (t->output.stderr != NULL)
+	if (t->output.std_err != NULL)
 		close(fderr[0]);
 	close(fdmonitor[0]);
 
@@ -650,14 +650,14 @@ int test_run(const struct test *t)
 	if (t->need_spawn && oneshot)
 		test_run_spawned(t);
 
-	if (t->output.stdout != NULL) {
+	if (t->output.std_out != NULL) {
 		if (pipe(fdout) != 0) {
 			ERR("could not create out pipe for %s\n", t->name);
 			return EXIT_FAILURE;
 		}
 	}
 
-	if (t->output.stderr != NULL) {
+	if (t->output.std_err != NULL) {
 		if (pipe(fderr) != 0) {
 			ERR("could not create err pipe for %s\n", t->name);
 			return EXIT_FAILURE;
diff --git a/testsuite/testsuite.h b/testsuite/testsuite.h
index 329d4a1..f8085a7 100644
--- a/testsuite/testsuite.h
+++ b/testsuite/testsuite.h
@@ -82,9 +82,9 @@ struct test {
 	const char *description;
 	struct {
 		/* File with correct stdout */
-		const char *stdout;
+		const char *std_out;
 		/* File with correct stderr */
-		const char *stderr;
+		const char *std_err;
 
 		/*
 		 * Vector with pair of files
-- 
1.7.3.4

Re: [PATCH 3/3] testsuite: fix usage of reserved names

[Lucas De Marchi]

On Mon, Aug 26, 2013 at 3:20 PM, John Spencer <maillist-kmod@barfooze.de> wrote:
> stdout and stderr are names reserved for the implementation
> and musl uses them rightfully as macro - and the expansion
> causes (of course) unexpected results.

couldn't musl just do what glibc does here?

<... define the types here ...>
#define stdin stdin
#define stdout stdout
#define stderr stderr

This would avoid patches like this to several projects.

>
> renaming the struct members stdout to std_out and stderr
> to std_err, to be 1) compliant 2) cause compilation to
> succeed.

uggh.. "out" and "err" otherwise. No need for the "std_" prefix.


Lucas De Marchi

[PATCH 3/3 v2] testsuite: fix usage of reserved names

[John Spencer]

stdout and stderr are names reserved for the implementation
and musl uses them rightfully as macro - and the expansion
causes (of course) unexpected results.

rename the struct members stdout to out and stderr
to err, to be 1) compliant 2) cause compilation to
succeed.

fixes build with musl libc.

Signed-off-by: John Spencer <maillist-kmod@barfooze.de>

---
 testsuite/test-alias.c      |    2 +-
 testsuite/test-loaded.c     |    2 +-
 testsuite/test-modinfo.c    |    2 +-
 testsuite/test-modprobe.c   |    8 ++++----
 testsuite/test-new-module.c |    4 ++--
 testsuite/testsuite.c       |   30 +++++++++++++++---------------
 testsuite/testsuite.h       |    4 ++--
 7 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/testsuite/test-alias.c b/testsuite/test-alias.c
index 5e21573..4d1d8d3 100644
--- a/testsuite/test-alias.c
+++ b/testsuite/test-alias.c
@@ -64,7 +64,7 @@ static DEFINE_TEST(alias_1,
 	},
 	.need_spawn = true,
 	.output = {
-		.stdout = TESTSUITE_ROOTFS "test-alias/correct.txt",
+		.out = TESTSUITE_ROOTFS "test-alias/correct.txt",
 	});
 
 static const struct test *tests[] = {
diff --git a/testsuite/test-loaded.c b/testsuite/test-loaded.c
index 33865cb..4b50813 100644
--- a/testsuite/test-loaded.c
+++ b/testsuite/test-loaded.c
@@ -85,7 +85,7 @@ static DEFINE_TEST(loaded_1,
 	},
 	.need_spawn = true,
 	.output = {
-		.stdout = TESTSUITE_ROOTFS "test-loaded/correct.txt",
+		.out = TESTSUITE_ROOTFS "test-loaded/correct.txt",
 	});
 
 static const struct test *tests[] = {
diff --git a/testsuite/test-modinfo.c b/testsuite/test-modinfo.c
index c5934ea..e372310 100644
--- a/testsuite/test-modinfo.c
+++ b/testsuite/test-modinfo.c
@@ -46,7 +46,7 @@ static DEFINE_TEST(modinfo_jonsmodules,
 		[TC_ROOTFS] = TESTSUITE_ROOTFS "test-modinfo/",
 	},
 	.output = {
-		.stdout = TESTSUITE_ROOTFS "test-modinfo/correct.txt",
+		.out = TESTSUITE_ROOTFS "test-modinfo/correct.txt",
 	});
 
 static const struct test *tests[] = {
diff --git a/testsuite/test-modprobe.c b/testsuite/test-modprobe.c
index f0131e3..0ae03a6 100644
--- a/testsuite/test-modprobe.c
+++ b/testsuite/test-modprobe.c
@@ -45,7 +45,7 @@ static DEFINE_TEST(modprobe_show_depends,
 		[TC_ROOTFS] = TESTSUITE_ROOTFS "test-modprobe/show-depends",
 	},
 	.output = {
-		.stdout = TESTSUITE_ROOTFS "test-modprobe/show-depends/correct.txt",
+		.out = TESTSUITE_ROOTFS "test-modprobe/show-depends/correct.txt",
 	});
 
 static __noreturn int modprobe_show_depends2(const struct test *t)
@@ -67,7 +67,7 @@ static DEFINE_TEST(modprobe_show_depends2,
 		[TC_ROOTFS] = TESTSUITE_ROOTFS "test-modprobe/show-depends",
 	},
 	.output = {
-		.stdout = TESTSUITE_ROOTFS "test-modprobe/show-depends/correct-psmouse.txt",
+		.out = TESTSUITE_ROOTFS "test-modprobe/show-depends/correct-psmouse.txt",
 	});
 
 
@@ -90,7 +90,7 @@ static DEFINE_TEST(modprobe_show_alias_to_none,
 		[TC_ROOTFS] = TESTSUITE_ROOTFS "test-modprobe/alias-to-none",
 	},
 	.output = {
-		.stdout = TESTSUITE_ROOTFS "test-modprobe/show-depends/correct-psmouse.txt",
+		.out = TESTSUITE_ROOTFS "test-modprobe/show-depends/correct-psmouse.txt",
 	});
 
 
@@ -177,7 +177,7 @@ static DEFINE_TEST(modprobe_param_kcmdline,
 		[TC_ROOTFS] = TESTSUITE_ROOTFS "test-modprobe/module-param-kcmdline",
 	},
 	.output = {
-		.stdout = TESTSUITE_ROOTFS "test-modprobe/module-param-kcmdline/correct.txt",
+		.out = TESTSUITE_ROOTFS "test-modprobe/module-param-kcmdline/correct.txt",
 	});
 
 
diff --git a/testsuite/test-new-module.c b/testsuite/test-new-module.c
index 240480e..0892507 100644
--- a/testsuite/test-new-module.c
+++ b/testsuite/test-new-module.c
@@ -67,7 +67,7 @@ static DEFINE_TEST(from_name,
 	},
 	.need_spawn = true,
 	.output = {
-		.stdout = TESTSUITE_ROOTFS "test-new-module/from_name/correct.txt",
+		.out = TESTSUITE_ROOTFS "test-new-module/from_name/correct.txt",
 	});
 
 static int from_alias(const struct test *t)
@@ -112,7 +112,7 @@ static DEFINE_TEST(from_alias,
 	},
 	.need_spawn = true,
 	.output = {
-		.stdout = TESTSUITE_ROOTFS "test-new-module/from_alias/correct.txt",
+		.out = TESTSUITE_ROOTFS "test-new-module/from_alias/correct.txt",
 	});
 
 static const struct test *tests[] = {
diff --git a/testsuite/testsuite.c b/testsuite/testsuite.c
index 6d85d7f..9877a64 100644
--- a/testsuite/testsuite.c
+++ b/testsuite/testsuite.c
@@ -223,7 +223,7 @@ static inline int test_run_child(const struct test *t, int fdout[2],
 	test_export_environ(t);
 
 	/* Close read-fds and redirect std{out,err} to the write-fds */
-	if (t->output.stdout != NULL) {
+	if (t->output.out != NULL) {
 		close(fdout[0]);
 		if (dup2(fdout[1], STDOUT_FILENO) < 0) {
 			ERR("could not redirect stdout to pipe: %m\n");
@@ -231,10 +231,10 @@ static inline int test_run_child(const struct test *t, int fdout[2],
 		}
 	}
 
-	if (t->output.stderr != NULL) {
+	if (t->output.err != NULL) {
 		close(fderr[0]);
 		if (dup2(fderr[1], STDERR_FILENO) < 0) {
-			ERR("could not redirect stdout to pipe: %m\n");
+			ERR("could not redirect stderr to pipe: %m\n");
 			exit(EXIT_FAILURE);
 		}
 	}
@@ -282,12 +282,12 @@ static inline bool test_run_parent_check_outputs(const struct test *t,
 		return false;
 	}
 
-	if (t->output.stdout != NULL) {
-		fd_matchout = open(t->output.stdout, O_RDONLY);
+	if (t->output.out != NULL) {
+		fd_matchout = open(t->output.out, O_RDONLY);
 		if (fd_matchout < 0) {
 			err = -errno;
 			ERR("could not open %s for read: %m\n",
-							t->output.stdout);
+							t->output.out);
 			goto out;
 		}
 		memset(&ep_outpipe, 0, sizeof(struct epoll_event));
@@ -301,12 +301,12 @@ static inline bool test_run_parent_check_outputs(const struct test *t,
 	} else
 		fdout = -1;
 
-	if (t->output.stderr != NULL) {
-		fd_matcherr = open(t->output.stderr, O_RDONLY);
+	if (t->output.err != NULL) {
+		fd_matcherr = open(t->output.err, O_RDONLY);
 		if (fd_matcherr < 0) {
 			err = -errno;
 			ERR("could not open %s for read: %m\n",
-					t->output.stderr);
+					t->output.err);
 			goto out;
 
 		}
@@ -536,9 +536,9 @@ static inline int test_run_parent(const struct test *t, int fdout[2],
 	bool matchout;
 
 	/* Close write-fds */
-	if (t->output.stdout != NULL)
+	if (t->output.out != NULL)
 		close(fdout[1]);
-	if (t->output.stderr != NULL)
+	if (t->output.err != NULL)
 		close(fderr[1]);
 	close(fdmonitor[1]);
 
@@ -549,9 +549,9 @@ static inline int test_run_parent(const struct test *t, int fdout[2],
 	 * break pipe on the other end: either child already closed or we want
 	 * to stop it
 	 */
-	if (t->output.stdout != NULL)
+	if (t->output.out != NULL)
 		close(fdout[0]);
-	if (t->output.stderr != NULL)
+	if (t->output.err != NULL)
 		close(fderr[0]);
 	close(fdmonitor[0]);
 
@@ -650,14 +650,14 @@ int test_run(const struct test *t)
 	if (t->need_spawn && oneshot)
 		test_run_spawned(t);
 
-	if (t->output.stdout != NULL) {
+	if (t->output.out != NULL) {
 		if (pipe(fdout) != 0) {
 			ERR("could not create out pipe for %s\n", t->name);
 			return EXIT_FAILURE;
 		}
 	}
 
-	if (t->output.stderr != NULL) {
+	if (t->output.err != NULL) {
 		if (pipe(fderr) != 0) {
 			ERR("could not create err pipe for %s\n", t->name);
 			return EXIT_FAILURE;
diff --git a/testsuite/testsuite.h b/testsuite/testsuite.h
index 329d4a1..e4f3ecf 100644
--- a/testsuite/testsuite.h
+++ b/testsuite/testsuite.h
@@ -82,9 +82,9 @@ struct test {
 	const char *description;
 	struct {
 		/* File with correct stdout */
-		const char *stdout;
+		const char *out;
 		/* File with correct stderr */
-		const char *stderr;
+		const char *err;
 
 		/*
 		 * Vector with pair of files
-- 
1.7.3.4

Re: [PATCH 3/3] testsuite: fix usage of reserved names

[John Spencer]

On 08/27/2013 01:07 AM, Lucas De Marchi wrote:
> On Mon, Aug 26, 2013 at 3:20 PM, John Spencer<maillist-kmod@barfooze.de>  wrote:
>> stdout and stderr are names reserved for the implementation
>> and musl uses them rightfully as macro - and the expansion
>> causes (of course) unexpected results.
>
> couldn't musl just do what glibc does here?

maybe, but often what glibc does is suboptimal or non-conforming, and 
relying on some  assumptions about how libc defines certain internal 
specifiers (possibly temporary) seems pretty fragile. even glibc may 
change its definition at some point to something that breaks kmod.

>
> <... define the types here ...>
> #define stdin stdin
> #define stdout stdout
> #define stderr stderr
>
> This would avoid patches like this to several projects.

i've ported about 500 packages to musl and this is first usage of stderr 
and stdout as variable names i've seen so far.

>
>>
>> renaming the struct members stdout to std_out and stderr
>> to std_err, to be 1) compliant 2) cause compilation to
>> succeed.
>
> uggh.. "out" and "err" otherwise. No need for the "std_" prefix.

right, that looks much nicer now. updated patch sent separately.

>
>
> Lucas De Marchi
>

thanks,
--JS

Re: [PATCH 3/3] testsuite: fix usage of reserved names

[Lucas De Marchi]

On Mon, Aug 26, 2013 at 3:20 PM, John Spencer <maillist-kmod@barfooze.de> wrote:
> stdout and stderr are names reserved for the implementation
> and musl uses them rightfully as macro - and the expansion
> causes (of course) unexpected results.
>
> renaming the struct members stdout to std_out and stderr
> to std_err, to be 1) compliant 2) cause compilation to
> succeed.
>
> fixes build with musl libc.
>
> Signed-off-by: John Spencer <maillist-kmod@barfooze.de>

We don't use signed-of-by in kmod.

Since this was the only issue I went ahead and applied the patch without it.


Thanks.

Lucas De Marchi

Re: [PATCH 1/3] remove non-portable usage of strndupa

[Kay Sievers]

On Mon, Aug 26, 2013 at 8:20 PM, John Spencer <maillist-kmod@barfooze.de> wrote:
> usage of strndupa is neither C99 nor POSIX,
> so musl libc does not implement it.
> it's a glibc invention, and a dangerous one since
> usage of alloca() is considered bad practice.

It's as dangerous as variable sized arrays, and surely much less "bad
practice" than your introduced PATH_MAX. :)

If kmod declares posix-only interfaces as the goal, then it's fine,
but your reasons do not make much sense in 2013.

Kay

Re: [PATCH 1/3] remove non-portable usage of strndupa

[Lucas De Marchi]

On Mon, Aug 26, 2013 at 3:20 PM, John Spencer <maillist-kmod@barfooze.de> wrote:
> usage of strndupa is neither C99 nor POSIX,
> so musl libc does not implement it.
> it's a glibc invention, and a dangerous one since
> usage of alloca() is considered bad practice.

If the input is already sanitized and checked for length, this is
isn't really dangerous. Particularly on recursive functions this also
avoids growing the stack much more than needed.

>
> fixes build with musl libc.

for me it seems more like an excuse to not implement it in musl.
Particularly because there are other places in which we call alloca(),
that you didn't complain. What does musl do there? If musl has
alloca(), doing strndupa in missing.h would be doable.

>
> Signed-off-by: John Spencer <maillist-kmod@barfooze.de>
>
> ---
>  libkmod/libkmod-util.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/libkmod/libkmod-util.c b/libkmod/libkmod-util.c
> index d686250..9615d0f 100644
> --- a/libkmod/libkmod-util.c
> +++ b/libkmod/libkmod-util.c
> @@ -28,6 +28,7 @@
>  #include <unistd.h>
>  #include <errno.h>
>  #include <string.h>
> +#include <limits.h>
>  #include <ctype.h>
>
>  #include "libkmod.h"
> @@ -323,8 +324,11 @@ static inline int is_dir(const char *path)
>  int mkdir_p(const char *path, int len, mode_t mode)
>  {
>         char *start, *end;
> -
> -       start = strndupa(path, len);
> +       char buf[PATH_MAX+1];
> +       snprintf(buf, sizeof buf, "%s", path);

snprintf to dup a string? You already know the len. memcpy would be way simpler

Lucas De Marchi

Re: [PATCH 1/3] remove non-portable usage of strndupa

[John Spencer]

On 08/27/2013 01:46 AM, Lucas De Marchi wrote:
> On Mon, Aug 26, 2013 at 3:20 PM, John Spencer<maillist-kmod@barfooze.de>  wrote:
>> usage of strndupa is neither C99 nor POSIX,
>> so musl libc does not implement it.
>> it's a glibc invention, and a dangerous one since
>> usage of alloca() is considered bad practice.
>
> If the input is already sanitized and checked for length, this is
> isn't really dangerous. Particularly on recursive functions this also
> avoids growing the stack much more than needed.

yes, but it is a dangerous function which can and will be misused when 
it is provided.

>
>>
>> fixes build with musl libc.
>
> for me it seems more like an excuse to not implement it in musl.
> Particularly because there are other places in which we call alloca(),
> that you didn't complain. What does musl do there? If musl has
> alloca(), doing strndupa in missing.h would be doable.

i just fixed strndupa usage in eudev's mkdir_p function a week ago, and 
that was the first usage of that function i've seen so far in ~500 
packages i've ported. the function used by kmod seems to be derived from 
that one, since the context, name and everything else are nearly identical.

musl usually doesn't add exotic functions only used by a single package, 
even moreso when the function in question can compromise security.

since kmod is not a red-hat-GNU-linux specific package like systemd 
(which is even proud of nonportable code), but a linux-specific one, 
portability at least among available linux libcs should be a concern.

>
>>
>> Signed-off-by: John Spencer<maillist-kmod@barfooze.de>
>>
>> ---
>>   libkmod/libkmod-util.c |    8 ++++++--
>>   1 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/libkmod/libkmod-util.c b/libkmod/libkmod-util.c
>> index d686250..9615d0f 100644
>> --- a/libkmod/libkmod-util.c
>> +++ b/libkmod/libkmod-util.c
>> @@ -28,6 +28,7 @@
>>   #include<unistd.h>
>>   #include<errno.h>
>>   #include<string.h>
>> +#include<limits.h>
>>   #include<ctype.h>
>>
>>   #include "libkmod.h"
>> @@ -323,8 +324,11 @@ static inline int is_dir(const char *path)
>>   int mkdir_p(const char *path, int len, mode_t mode)
>>   {
>>          char *start, *end;
>> -
>> -       start = strndupa(path, len);
>> +       char buf[PATH_MAX+1];
>> +       snprintf(buf, sizeof buf, "%s", path);
>
> snprintf to dup a string? You already know the len. memcpy would be way simpler

this is to cover the potential case where len > strlen(path).
there's no documentation indicating this case cannot happen.
otherwise an out-of-bounds read would happen which could potentially 
cause a segfault. i suspect this function is not called million of times 
in an inner loop, so i opted for security over speed.

>
> Lucas De Marchi
>

Re: [PATCH 1/3] remove non-portable usage of strndupa

[Kay Sievers]

On Tue, Aug 27, 2013 at 2:16 AM, John Spencer <maillist-kmod@barfooze.de> wrote:
> On 08/27/2013 01:46 AM, Lucas De Marchi wrote:
>>
>> On Mon, Aug 26, 2013 at 3:20 PM, John Spencer<maillist-kmod@barfooze.de>
>> wrote:
>>>
>>> usage of strndupa is neither C99 nor POSIX,
>>> so musl libc does not implement it.
>>> it's a glibc invention, and a dangerous one since
>>> usage of alloca() is considered bad practice.
>>
>>
>> If the input is already sanitized and checked for length, this is
>> isn't really dangerous. Particularly on recursive functions this also
>> avoids growing the stack much more than needed.
>
> yes, but it is a dangerous function which can and will be misused when it is
> provided.

Dangerous? Misused? That is just pure nonsense.

> musl usually doesn't add exotic functions only used by a single package,
> even moreso when the function in question can compromise security.

So, this exotic libc can't be used for kmod as it is. Fine. :)

> since kmod is not a red-hat-GNU-linux specific package like systemd (which
> is even proud of nonportable code), but a linux-specific one, portability at
> least among available linux libcs should be a concern.

Just a hint: I would try to be less confrontational, if you like stuff
to be changed in a way that matters only to you.

Kay

Re: [PATCH 1/3] remove non-portable usage of strndupa

[Lucas De Marchi]

On Mon, Aug 26, 2013 at 9:16 PM, John Spencer <maillist-kmod@barfooze.de> wrote:
> On 08/27/2013 01:46 AM, Lucas De Marchi wrote:
>>
>> On Mon, Aug 26, 2013 at 3:20 PM, John Spencer<maillist-kmod@barfooze.de>
>> wrote:
>>>
>>> usage of strndupa is neither C99 nor POSIX,
>>> so musl libc does not implement it.
>>> it's a glibc invention, and a dangerous one since
>>> usage of alloca() is considered bad practice.
>>
>>
>> If the input is already sanitized and checked for length, this is
>> isn't really dangerous. Particularly on recursive functions this also
>> avoids growing the stack much more than needed.
>
>
> yes, but it is a dangerous function which can and will be misused when it is
> provided.

If I wanted a language that doesn't allow me to shoot myself on the
foot, I'd rather be writing pascal,  basic or some other new
<name-your-preferred-script-language>

>
>
>>
>>>
>>> fixes build with musl libc.
>>
>>
>> for me it seems more like an excuse to not implement it in musl.
>> Particularly because there are other places in which we call alloca(),
>> that you didn't complain. What does musl do there? If musl has
>> alloca(), doing strndupa in missing.h would be doable.
>
>
> i just fixed strndupa usage in eudev's mkdir_p function a week ago, and that
> was the first usage of that function i've seen so far in ~500 packages i've
> ported. the function used by kmod seems to be derived from that one, since
> the context, name and everything else are nearly identical.

apart from the name, I don't think it's derived

>
> musl usually doesn't add exotic functions only used by a single package,

if you keep removing it from the packages, you may decrease the number
of users.... but counting now you just said you have 2. And I can name
some others, too.

> even moreso when the function in question can compromise security.
>
> since kmod is not a red-hat-GNU-linux specific package like systemd (which
> is even proud of nonportable code), but a linux-specific one, portability at
> least among available linux libcs should be a concern.

yes, I care. I do care other packages to be high quality too instead
of just throwing into kmod an insane amount of ifdefs and friends to
support everything. In the past this worked very well and allowed us
to move forward faster and link to bionic, dielibc, klibc and musl. I
appreciate patches adding support to other libc, but they have to be
reasonable.


>
>
>>
>>>
>>> Signed-off-by: John Spencer<maillist-kmod@barfooze.de>
>>>
>>> ---
>>>   libkmod/libkmod-util.c |    8 ++++++--
>>>   1 files changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libkmod/libkmod-util.c b/libkmod/libkmod-util.c
>>> index d686250..9615d0f 100644
>>> --- a/libkmod/libkmod-util.c
>>> +++ b/libkmod/libkmod-util.c
>>> @@ -28,6 +28,7 @@
>>>   #include<unistd.h>
>>>   #include<errno.h>
>>>   #include<string.h>
>>> +#include<limits.h>
>>>   #include<ctype.h>
>>>
>>>   #include "libkmod.h"
>>> @@ -323,8 +324,11 @@ static inline int is_dir(const char *path)
>>>   int mkdir_p(const char *path, int len, mode_t mode)
>>>   {
>>>          char *start, *end;
>>> -
>>> -       start = strndupa(path, len);
>>> +       char buf[PATH_MAX+1];
>>> +       snprintf(buf, sizeof buf, "%s", path);
>>
>>
>> snprintf to dup a string? You already know the len. memcpy would be way
>> simpler
>
>
> this is to cover the potential case where len > strlen(path).
> there's no documentation indicating this case cannot happen.

there are only 2 callers of this function. Copying the entire string
here is actually the wrong thing to do.

Back to the subject, musl does provide alloca(), which is the
"dangerous" function according to you. If you or musl devs don't want
to provide strndupa(), the patch to be accepted in kmod is the one
that checks if strndupa() is available and add it to missing.h
otherwise. Why missing.h? To remember me to check it some time in
future to see if I can already remove some stuff.


Lucas De Marchi

Re: [PATCH 1/3] remove non-portable usage of strndupa

[John Spencer]

On 08/27/2013 05:35 AM, Lucas De Marchi wrote:
> On Mon, Aug 26, 2013 at 9:16 PM, John Spencer<maillist-kmod@barfooze.de>  wrote:
>> On 08/27/2013 01:46 AM, Lucas De Marchi wrote:
>>>
>>> On Mon, Aug 26, 2013 at 3:20 PM, John Spencer<maillist-kmod@barfooze.de>
>>> wrote:
>>>>
>>>> usage of strndupa is neither C99 nor POSIX,
>>>> so musl libc does not implement it.
>>>> it's a glibc invention, and a dangerous one since
>>>> usage of alloca() is considered bad practice.
>>>
>>>
>>> If the input is already sanitized and checked for length, this is
>>> isn't really dangerous. Particularly on recursive functions this also
>>> avoids growing the stack much more than needed.
>>
>>
>> yes, but it is a dangerous function which can and will be misused when it is
>> provided.
>
> If I wanted a language that doesn't allow me to shoot myself on the
> foot, I'd rather be writing pascal,  basic or some other new
> <name-your-preferred-script-language>
>

yes, but there's a reason alloca is not in the C standard and gets() got 
deprecated.

>>
>>
>>>
>>>>
>>>> fixes build with musl libc.
>>>
>>>
>>> for me it seems more like an excuse to not implement it in musl.
>>> Particularly because there are other places in which we call alloca(),
>>> that you didn't complain. What does musl do there? If musl has
>>> alloca(), doing strndupa in missing.h would be doable.
>>
>>
>> i just fixed strndupa usage in eudev's mkdir_p function a week ago, and that
>> was the first usage of that function i've seen so far in ~500 packages i've
>> ported. the function used by kmod seems to be derived from that one, since
>> the context, name and everything else are nearly identical.
>
> apart from the name, I don't think it's derived
>
>>
>> musl usually doesn't add exotic functions only used by a single package,
>
> if you keep removing it from the packages, you may decrease the number
> of users.... but counting now you just said you have 2. And I can name
> some others, too.

no, i have one, since eudev merged my patch without turning a hair.
they didn't fight around just to keep their single strndupa call in some 
legacy systemd derived code.
the others you can name are most likely udev and systemd, which i don't 
intend to ever compile, same as pulseaudio and other poettering crapware.

>
>> even moreso when the function in question can compromise security.
>>
>> since kmod is not a red-hat-GNU-linux specific package like systemd (which
>> is even proud of nonportable code), but a linux-specific one, portability at
>> least among available linux libcs should be a concern.
>
> yes, I care. I do care other packages to be high quality too instead
> of just throwing into kmod an insane amount of ifdefs and friends to
> support everything. In the past this worked very well and allowed us
> to move forward faster and link to bionic, dielibc, klibc and musl. I
> appreciate patches adding support to other libc, but they have to be
> reasonable.
>

high quality, high portability, no ifdefs - this is exactly what the 
proposed patch here does.

what exactly do you find non-reasonable regarding this patch ?

kmod used to build just fine in the past, you're the one who broke it 
last month by throwing the mkdir_p code into the source tree.
and the function has seen quite some refactoring already, using strdupa, 
then strndupa. who guarantees me that if i add strndupa (assuming rich 
would accept and merge my patch) to musl, you won't change your code 
back to use strdupa in the next release ?

>
>>
>>
>>>
>>>>
>>>> Signed-off-by: John Spencer<maillist-kmod@barfooze.de>
>>>>
>>>> ---
>>>>    libkmod/libkmod-util.c |    8 ++++++--
>>>>    1 files changed, 6 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/libkmod/libkmod-util.c b/libkmod/libkmod-util.c
>>>> index d686250..9615d0f 100644
>>>> --- a/libkmod/libkmod-util.c
>>>> +++ b/libkmod/libkmod-util.c
>>>> @@ -28,6 +28,7 @@
>>>>    #include<unistd.h>
>>>>    #include<errno.h>
>>>>    #include<string.h>
>>>> +#include<limits.h>
>>>>    #include<ctype.h>
>>>>
>>>>    #include "libkmod.h"
>>>> @@ -323,8 +324,11 @@ static inline int is_dir(const char *path)
>>>>    int mkdir_p(const char *path, int len, mode_t mode)
>>>>    {
>>>>           char *start, *end;
>>>> -
>>>> -       start = strndupa(path, len);
>>>> +       char buf[PATH_MAX+1];
>>>> +       snprintf(buf, sizeof buf, "%s", path);
>>>
>>>
>>> snprintf to dup a string? You already know the len. memcpy would be way
>>> simpler
>>
>>
>> this is to cover the potential case where len>  strlen(path).
>> there's no documentation indicating this case cannot happen.
>
> there are only 2 callers of this function. Copying the entire string
> here is actually the wrong thing to do.
>
> Back to the subject, musl does provide alloca(), which is the
> "dangerous" function according to you. If you or musl devs don't want
> to provide strndupa(), the patch to be accepted in kmod is the one
> that checks if strndupa() is available and add it to missing.h
> otherwise. Why missing.h? To remember me to check it some time in
> future to see if I can already remove some stuff.

actually i didn't even ask yet if we can ask strndupa, because it was 
first encountered a week back in said eudev code, and my patch got 
immediately merged.
when i see non-portable code, i fix the nonportable code, not libc, and 
send a patch to upstream in the hope that it gets merged. if they don't 
merge it, ok - yet another fork caused by maintainer stubborness.
how do you like "kemod" as a name for the fork ?

yes, alloca() was added as it is widely used - you can't even build gcc 
without it. the same can't be said about strndupa which is only used in 
one mkdir code snippet copy-pasted from udev into a handful of projects.

besides, afaik strndupa can't even be implemented as a function, since 
it would return a pointer to automatic storage that's already out of 
scope (at least not without using asm).

that said, i am not sending you a macro replacement for a missing 
strndupa and doing a full stop now. if you care about the musl userbase, 
merge this patch, otherwise ignore it and live with a fork.

>
>
> Lucas De Marchi
>

P.S. i don't even use kmod myself (contrary to what poettering's twin 
sievers believes), i ported this quickly for usage in dragora linux 
which is just switching from glibc to musl and the maintainer asked for 
help.
there will soon be a musl-based gentoo as well btw, and next version of 
alpine will be musl based as well.

Re: [PATCH 1/3] remove non-portable usage of strndupa

[Lucas De Marchi]

Hi John,

On Tue, Aug 27, 2013 at 6:04 AM, John Spencer <maillist-kmod@barfooze.de> wrote:
> On 08/27/2013 05:35 AM, Lucas De Marchi wrote:
>>
>> On Mon, Aug 26, 2013 at 9:16 PM, John Spencer<maillist-kmod@barfooze.de>
>> wrote:
>>>
>>> On 08/27/2013 01:46 AM, Lucas De Marchi wrote:
>>>>
>>>>
>>>> On Mon, Aug 26, 2013 at 3:20 PM, John Spencer<maillist-kmod@barfooze.de>
>>>> wrote:
>>>>>
>>>>>
>>>>> usage of strndupa is neither C99 nor POSIX,
>>>>> so musl libc does not implement it.
>>>>> it's a glibc invention, and a dangerous one since
>>>>> usage of alloca() is considered bad practice.
>>>>
>>>>
>>>>
>>>> If the input is already sanitized and checked for length, this is
>>>> isn't really dangerous. Particularly on recursive functions this also
>>>> avoids growing the stack much more than needed.
>>>
>>>
>>>
>>> yes, but it is a dangerous function which can and will be misused when it
>>> is
>>> provided.
>>
>>
>> If I wanted a language that doesn't allow me to shoot myself on the
>> foot, I'd rather be writing pascal,  basic or some other new
>> <name-your-preferred-script-language>
>>
>
> yes, but there's a reason alloca is not in the C standard and gets() got
> deprecated.

the thing is... a standard is a live thing. New things are added.
Other things are removed. gets() was removed because there's no way to
use it correctly. New things are being added because people need and
because we are getting innovative things being done.

If we were to limit ourselves only to things that were already in a
standard, the standard would never need to be updated and how boring
would be life.


>
>
>>>
>>>
>>>>
>>>>>
>>>>> fixes build with musl libc.
>>>>
>>>>
>>>>
>>>> for me it seems more like an excuse to not implement it in musl.
>>>> Particularly because there are other places in which we call alloca(),
>>>> that you didn't complain. What does musl do there? If musl has
>>>> alloca(), doing strndupa in missing.h would be doable.
>>>
>>>
>>>
>>> i just fixed strndupa usage in eudev's mkdir_p function a week ago, and
>>> that
>>> was the first usage of that function i've seen so far in ~500 packages
>>> i've
>>> ported. the function used by kmod seems to be derived from that one,
>>> since
>>> the context, name and everything else are nearly identical.
>>
>>
>> apart from the name, I don't think it's derived
>>
>>>
>>> musl usually doesn't add exotic functions only used by a single package,
>>
>>
>> if you keep removing it from the packages, you may decrease the number
>> of users.... but counting now you just said you have 2. And I can name
>> some others, too.
>
>
> no, i have one, since eudev merged my patch without turning a hair.
> they didn't fight around just to keep their single strndupa call in some
> legacy systemd derived code.
> the others you can name are most likely udev and systemd, which i don't
> intend to ever compile, same as pulseaudio and other poettering crapware.


some projects are different and reviewers are picker about the changes
merged. If you are aiming to make several projects to build correctly
with musl, the best thing you do is to adapt yourself to the different
projects.

Strong words and bashing other projects won't help your cause.

>
>
>>
>>> even moreso when the function in question can compromise security.
>>>
>>> since kmod is not a red-hat-GNU-linux specific package like systemd
>>> (which
>>> is even proud of nonportable code), but a linux-specific one, portability
>>> at
>>> least among available linux libcs should be a concern.
>>
>>
>> yes, I care. I do care other packages to be high quality too instead
>> of just throwing into kmod an insane amount of ifdefs and friends to
>> support everything. In the past this worked very well and allowed us
>> to move forward faster and link to bionic, dielibc, klibc and musl. I
>> appreciate patches adding support to other libc, but they have to be
>> reasonable.
>>
>
> high quality, high portability, no ifdefs - this is exactly what the
> proposed patch here does.
>
> what exactly do you find non-reasonable regarding this patch ?

Because we/I like extensions and other projects could be improved by
not getting stuck on old things. GCC was improved because of that.
LLVM was improved because of that. Several libc's were improved
because of that.

I don't really mind if that particular piece of code is changed to a
fixed array. That function could be refactored to even not duplicate
each part of the path. However what I really want is people to *stop*
and *think* if other projects like musl could be improved instead of
just changing that piece of code.

For example, you didn't properly respond this particular question: if
musl already provides alloca(), why can't it provide str[n]dupa()? It
could be trivially done by a macro or static always inline function.

>
> kmod used to build just fine in the past, you're the one who broke it last
> month by throwing the mkdir_p code into the source tree.
> and the function has seen quite some refactoring already, using strdupa,
> then strndupa. who guarantees me that if i add strndupa (assuming rich would
> accept and merge my patch) to musl, you won't change your code back to use
> strdupa in the next release ?

None. And I can't guarantee you next release will be bug free neither.
 I don't use musl myself as I don't use plenty of other libc's or
architectures in which kmod runs on. This is one reason why we
appreciate bug reports and patches from others


>
>
>>
>>>
>>>
>>>>
>>>>>
>>>>> Signed-off-by: John Spencer<maillist-kmod@barfooze.de>
>>>>>
>>>>> ---
>>>>>    libkmod/libkmod-util.c |    8 ++++++--
>>>>>    1 files changed, 6 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/libkmod/libkmod-util.c b/libkmod/libkmod-util.c
>>>>> index d686250..9615d0f 100644
>>>>> --- a/libkmod/libkmod-util.c
>>>>> +++ b/libkmod/libkmod-util.c
>>>>> @@ -28,6 +28,7 @@
>>>>>    #include<unistd.h>
>>>>>    #include<errno.h>
>>>>>    #include<string.h>
>>>>> +#include<limits.h>
>>>>>    #include<ctype.h>
>>>>>
>>>>>    #include "libkmod.h"
>>>>> @@ -323,8 +324,11 @@ static inline int is_dir(const char *path)
>>>>>    int mkdir_p(const char *path, int len, mode_t mode)
>>>>>    {
>>>>>           char *start, *end;
>>>>> -
>>>>> -       start = strndupa(path, len);
>>>>> +       char buf[PATH_MAX+1];
>>>>> +       snprintf(buf, sizeof buf, "%s", path);
>>>>
>>>>
>>>>
>>>> snprintf to dup a string? You already know the len. memcpy would be way
>>>> simpler
>>>
>>>
>>>
>>> this is to cover the potential case where len>  strlen(path).
>>> there's no documentation indicating this case cannot happen.
>>
>>
>> there are only 2 callers of this function. Copying the entire string
>> here is actually the wrong thing to do.
>>
>> Back to the subject, musl does provide alloca(), which is the
>> "dangerous" function according to you. If you or musl devs don't want
>> to provide strndupa(), the patch to be accepted in kmod is the one
>> that checks if strndupa() is available and add it to missing.h
>> otherwise. Why missing.h? To remember me to check it some time in
>> future to see if I can already remove some stuff.
>
>
> actually i didn't even ask yet if we can ask strndupa, because it was first
> encountered a week back in said eudev code, and my patch got immediately
> merged.
> when i see non-portable code, i fix the nonportable code, not libc, and send
> a patch to upstream in the hope that it gets merged. if they don't merge it,
> ok - yet another fork caused by maintainer stubborness.

Again, getting personal is not helping your cause.

> how do you like "kemod" as a name for the fork ?

And neither threatening like this

>
> yes, alloca() was added as it is widely used - you can't even build gcc
> without it. the same can't be said about strndupa which is only used in one
> mkdir code snippet copy-pasted from udev into a handful of projects.

you should grep more code.

>
> besides, afaik strndupa can't even be implemented as a function, since it
> would return a pointer to automatic storage that's already out of scope (at
> least not without using asm).

See reply above.

>
> that said, i am not sending you a macro replacement for a missing strndupa
> and doing a full stop now. if you care about the musl userbase, merge this
> patch, otherwise ignore it and live with a fork.
>
>>
>>
>> Lucas De Marchi
>>
>
> P.S. i don't even use kmod myself (contrary to what poettering's twin
> sievers believes), i ported this quickly for usage in dragora linux which is

Enough of that. Please refrain to insult people on this mailing list.


Lucas De Marchi