Skip to contentSkip to conversation

util-linux: unshare: include libmount.h to provide missing MS_* defines

PATCH New Romain Naour 2 participants 5 messages Apr 6, 2014

lore
Share & tools
lore

Romain Naour

author
#0 lore raw

PATCH 1/2 util-linux: unshare: include libmount.h to provide missing MS_* defines

upstream patch: d754315c54af9cb8222e0a04ed5b2b4b927ed176

Signed-off-by Romain Naour <[email protected]>
---
 ...util-linux-003-unshare-include-libmount.h.patch | 44 ++++++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 package/util-linux/util-linux-003-unshare-include-libmount.h.patch
package/util-linux/util-linux-003-unshare-include-libmount.h.patch new file +44 -0
new file mode 100644
index 0000000..5ccfe7d
@@ -0,0 +1,44 @@
+From d754315c54af9cb8222e0a04ed5b2b4b927ed176 Mon Sep 17 00:00:00 2001
+From: Ruediger Meier <[email protected]>
+Date: Mon, 17 Mar 2014 11:48:47 +0100
+Subject: [PATCH 1/1] unshare: include libmount.h to provide missing MS_*
+ defines
+
+Since 6728ca10 we are using MS_PRIVATE and MS_REC which are not defined
+in some systems's sys/mount.h.
+
+Signed-off-by: Ruediger Meier <[email protected]>
+---
+ sys-utils/Makemodule.am | 1 +
+ sys-utils/unshare.c     | 3 +++
+ 2 files changed, 4 insertions(+)
+
+diff --git a/sys-utils/Makemodule.am b/sys-utils/Makemodule.am
+index 30b5d3e..93a2c8d 100644
+--- a/sys-utils/Makemodule.am
++++ b/sys-utils/Makemodule.am
+@@ -296,6 +296,7 @@ usrbin_exec_PROGRAMS += unshare
+ dist_man_MANS += sys-utils/unshare.1
+ unshare_SOURCES = sys-utils/unshare.c
+ unshare_LDADD = $(LDADD) libcommon.la
++unshare_CFLAGS = $(AM_CFLAGS) -I$(ul_libmount_incdir)
+ endif
+ 
+ if BUILD_NSENTER
+diff --git a/sys-utils/unshare.c b/sys-utils/unshare.c
+index 91e0ec7..1240293 100644
+--- a/sys-utils/unshare.c
++++ b/sys-utils/unshare.c
+@@ -27,6 +27,9 @@
+ #include <sys/wait.h>
+ #include <sys/mount.h>
+ 
++/* we only need some defines missing in sys/mount.h, no libmount linkage */
++#include <libmount.h>
++
+ #include "nls.h"
+ #include "c.h"
+ #include "closestream.h"
+-- 
+1.9.0
+
Signature · 1 line
1.9.0
author
#1 lore raw

PATCH 2/2 util-linux: define mkostemp for uClibc

uclibc does not implement mkostemp GNU extension

Signed-off-by Romain Naour <[email protected]>
---
 ...util-linux-004-define-mkostemp-for-uClibc.patch | 29 ++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 package/util-linux/util-linux-004-define-mkostemp-for-uClibc.patch
package/util-linux/util-linux-004-define-mkostemp-for-uClibc.patch new file +29 -0
new file mode 100644
index 0000000..7b49d37
@@ -0,0 +1,29 @@
+From d339af8fdc4b28175d6986d870f6735a9c2e90a3 Mon Sep 17 00:00:00 2001
+From: Romain Naour <[email protected]>
+Date: Sun, 6 Apr 2014 01:48:58 +0200
+Subject: [PATCH 1/1] c.h: define mkostemp for uClibc
+
+uclibc does not implement mkostemp GNU extension
+
+Signed-off-by: Romain Naour <[email protected]>
+---
+ include/c.h | 5 +++++
+ 1 file changed, 5 insertions(+)
+
+diff --git a/include/c.h b/include/c.h
+index 4a9bf3d..3ba51c1 100644
+--- a/include/c.h
++++ b/include/c.h
+@@ -300,4 +300,9 @@ static inline int usleep(useconds_t usec)
+ # define SEEK_HOLE	4
+ #endif
+ 
++#ifdef __UCLIBC__
++/* uclibc does not implement mkostemp GNU extension */
++#define mkostemp(x,y) mkstemp(x)
++#endif
++
+ #endif /* UTIL_LINUX_C_H */
+-- 
+1.9.0
+
Signature · 1 line
1.9.0
#2 lore raw

PATCH 2/2 util-linux: define mkostemp for uClibc

Dear Romain Naour,

Quoted 20 lines · Romain Naour
package/util-linux/util-linux-004-define-mkostemp-for-uClibc.patch new file +14 -0
new file mode 100644
index 0000000..7b49d37
@@ -0,0 +1,29 @@
+From d339af8fdc4b28175d6986d870f6735a9c2e90a3 Mon Sep 17 00:00:00 2001
+From: Romain Naour <[email protected]>
+Date: Sun, 6 Apr 2014 01:48:58 +0200
+Subject: [PATCH 1/1] c.h: define mkostemp for uClibc
+
+uclibc does not implement mkostemp GNU extension
+
+Signed-off-by: Romain Naour <[email protected]>
+---
+ include/c.h | 5 +++++
+ 1 file changed, 5 insertions(+)
+
+diff --git a/include/c.h b/include/c.h
+index 4a9bf3d..3ba51c1 100644

package/util-linux/util-linux-004-define-mkostemp-for-uClibc.patch line 24

Quoted 10 lines
package/util-linux/util-linux-004-define-mkostemp-for-uClibc.patch new file +10 -0
+--- a/include/c.h
++++ b/include/c.h
+@@ -300,4 +300,9 @@ static inline int usleep(useconds_t usec)
+ # define SEEK_HOLE	4
+ #endif
+ 
++#ifdef __UCLIBC__
++/* uclibc does not implement mkostemp GNU extension */
++#define mkostemp(x,y) mkstemp(x)
++#endif

Thanks for the patch. However, I believe there are two problems here:

1/ mkostemp() *can* exist with uClibc: the master branch of uClibc,
which we use for some architectures (ARC and Xtensa) do have mkostemp(). Therefore, there should instead be a test in configure.ac like AC_CHECK_FUNCS([mkostemp]) and then make your code conditional on #ifndef HAVE_MKOSTEMP.

2/ The other problem is that in util-linux, mkostemp is used as
follows:

fd = mkostemp(n, O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC);

In your implementation, you discard the flags entirely, by using mkstemp(). mkstemp() will pass O_RDWR|O_CREAT|O_EXCL, but not O_CLOEXEC, which means that the file descriptor will no longer be closed automatically upon exec(). Though, looking at
http://git.kernel.org/cgit/utils/util-linux/util-linux.git/commit/?id=b1fa3e2234fab95960eaa8499384000f189def13,
they have switched to using O_CLOEXEC with no specific reason. Maybe I'm just worrying too much. But a solution might be to do something like:

#ifndef HAVE_MKOSTEMP
static inline int mkostemp(char *template, int flags)

{
	int fd;
	fd = mkstemp(template);
	if (flags & O_CLOEXEC && fd >= 0)
		fcntl(fd, F_SETFD, FD_CLOEXEC);
	return fd;
}

#endif

(Completely untested, not even compiled)

Don't know if we want to worry about this though.

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
author
#3 lore raw

PATCH 2/2 util-linux: define mkostemp for uClibc

Hi Thomas,

Le 06/04/2014 11:45, Thomas Petazzoni a ?crit :

Quoted 36 lines

Dear Romain Naour,

Quoted 20 lines · Romain Naour
package/util-linux/util-linux-004-define-mkostemp-for-uClibc.patch new file +14 -0
new file mode 100644
index 0000000..7b49d37
@@ -0,0 +1,29 @@
+From d339af8fdc4b28175d6986d870f6735a9c2e90a3 Mon Sep 17 00:00:00 2001
+From: Romain Naour <[email protected]>
+Date: Sun, 6 Apr 2014 01:48:58 +0200
+Subject: [PATCH 1/1] c.h: define mkostemp for uClibc
+
+uclibc does not implement mkostemp GNU extension
+
+Signed-off-by: Romain Naour <[email protected]>
+---
+ include/c.h | 5 +++++
+ 1 file changed, 5 insertions(+)
+
+diff --git a/include/c.h b/include/c.h
+index 4a9bf3d..3ba51c1 100644

package/util-linux/util-linux-004-define-mkostemp-for-uClibc.patch line 24

Quoted 10 lines
package/util-linux/util-linux-004-define-mkostemp-for-uClibc.patch new file +10 -0
+--- a/include/c.h
++++ b/include/c.h
+@@ -300,4 +300,9 @@ static inline int usleep(useconds_t usec)
+ # define SEEK_HOLE	4
+ #endif
+
++#ifdef __UCLIBC__
++/* uclibc does not implement mkostemp GNU extension */
++#define mkostemp(x,y) mkstemp(x)
++#endif

Thanks for the patch. However, I believe there are two problems here:

Quoted 5 lines

1/ mkostemp() *can* exist with uClibc: the master branch of uClibc,
which we use for some architectures (ARC and Xtensa) do have mkostemp(). Therefore, there should instead be a test in configure.ac like AC_CHECK_FUNCS([mkostemp]) and then make your code conditional on #ifndef HAVE_MKOSTEMP.

Ok, I didn't notice that mkostemp() is available with latest uClibc, thanks.

Quoted 24 lines

2/ The other problem is that in util-linux, mkostemp is used as
follows:

fd = mkostemp(n, O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC);

In your implementation, you discard the flags entirely, by using mkstemp(). mkstemp() will pass O_RDWR|O_CREAT|O_EXCL, but not O_CLOEXEC, which means that the file descriptor will no longer be closed automatically upon exec(). Though, looking at
http://git.kernel.org/cgit/utils/util-linux/util-linux.git/commit/?id=b1fa3e2234fab95960eaa8499384000f189def13,
they have switched to using O_CLOEXEC with no specific reason. Maybe I'm just worrying too much. But a solution might be to do something like:

#ifndef HAVE_MKOSTEMP
static inline int mkostemp(char *template, int flags)

{
	int fd;
	fd = mkstemp(template);
	if (flags & O_CLOEXEC && fd >= 0)
		fcntl(fd, F_SETFD, FD_CLOEXEC);
	return fd;
}
Quoted 6 lines

#endif

(Completely untested, not even compiled)

Don't know if we want to worry about this though.

I had a build failure with the toolchain used by the autobuilder:
ctng-i386-pentium4-uclibc.tar.bz2
That why I added this patch.

Patch is on the way.

Best regards,
Romain

Applied
#4 lore raw

PATCH 1/2 util-linux: unshare: include libmount.h to provide missing MS_* defines

In reply to Romain Naour

Dear Romain Naour,

Quoted 7 lines · Romain Naour

upstream patch: d754315c54af9cb8222e0a04ed5b2b4b927ed176

Signed-off-by Romain Naour <[email protected]>
---
 ...util-linux-003-unshare-include-libmount.h.patch | 44 ++++++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 package/util-linux/util-linux-003-unshare-include-libmount.h.patch

Applied, thanks.

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com