[libvirt] [PATCH] build: work around FreeBSD stdlib.h bug
POSIX requires that <stdlib.h> expose WIFEXITED and friends, but FreeBSD and others fail to comply. We can work around it manually by including <sys/wait.h>, or we can work around it automatically by using gnulib's system-posix module. * bootstrap.conf (gnulib_modules): Add system-posix. Signed-off-by: Eric Blake <eblake@redhat.com> --- I'll wait for a review on this one - we don't use system(), and it feels a little bit odd to list the system() module merely for its side effect of a fixed <stdlib.h>. The alternative is to continue using <sys/wait.h> everywhere that we use WIFEXITED and friends. bootstrap.conf | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bootstrap.conf b/bootstrap.conf index d24a714..bc6ce80 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -1,6 +1,6 @@ # Bootstrap configuration. -# Copyright (C) 2010-2013 Red Hat, Inc. +# Copyright (C) 2010-2014 Red Hat, Inc. # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -112,6 +112,7 @@ strerror_r-posix strptime strsep strtok_r +system-posix sys_stat sys_wait termios -- 1.8.5.3
On Tue, Mar 04, 2014 at 06:28:17AM -0700, Eric Blake wrote:
POSIX requires that <stdlib.h> expose WIFEXITED and friends, but FreeBSD and others fail to comply. We can work around it manually by including <sys/wait.h>, or we can work around it automatically by using gnulib's system-posix module.
* bootstrap.conf (gnulib_modules): Add system-posix.
Signed-off-by: Eric Blake <eblake@redhat.com> ---
I'll wait for a review on this one - we don't use system(), and it feels a little bit odd to list the system() module merely for its side effect of a fixed <stdlib.h>. The alternative is to continue using <sys/wait.h> everywhere that we use WIFEXITED and friends.
Perhaps we should have a helper in util/virprocess.{c,h} for this, so no code outside that file ever need use WIFEXITED ? Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 03/04/2014 06:47 AM, Daniel P. Berrange wrote:
On Tue, Mar 04, 2014 at 06:28:17AM -0700, Eric Blake wrote:
POSIX requires that <stdlib.h> expose WIFEXITED and friends, but FreeBSD and others fail to comply. We can work around it manually by including <sys/wait.h>, or we can work around it automatically by using gnulib's system-posix module.
* bootstrap.conf (gnulib_modules): Add system-posix.
Signed-off-by: Eric Blake <eblake@redhat.com> ---
I'll wait for a review on this one - we don't use system(), and it feels a little bit odd to list the system() module merely for its side effect of a fixed <stdlib.h>. The alternative is to continue using <sys/wait.h> everywhere that we use WIFEXITED and friends.
Perhaps we should have a helper in util/virprocess.{c,h} for this, so no code outside that file ever need use WIFEXITED ?
We already have a couple of helper functions, and my recent virFork cleanups got rid of even more clients of WIFEXITED (that is, virCommandRun now defaults to returning sanitized rather than raw exit values). At this point, there are very few reasons for any new code to need to use WIFEXITED; it's mostly limited to existing code (but where my virFork cleanups tripped up on the FreeBSD header bug due to refactoring). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 03/04/2014 06:59 AM, Eric Blake wrote:
On 03/04/2014 06:47 AM, Daniel P. Berrange wrote:
On Tue, Mar 04, 2014 at 06:28:17AM -0700, Eric Blake wrote:
POSIX requires that <stdlib.h> expose WIFEXITED and friends, but FreeBSD and others fail to comply. We can work around it manually by including <sys/wait.h>, or we can work around it automatically by using gnulib's system-posix module.
* bootstrap.conf (gnulib_modules): Add system-posix.
Signed-off-by: Eric Blake <eblake@redhat.com> ---
I'll wait for a review on this one - we don't use system(), and it feels a little bit odd to list the system() module merely for its side effect of a fixed <stdlib.h>. The alternative is to continue using <sys/wait.h> everywhere that we use WIFEXITED and friends.
Perhaps we should have a helper in util/virprocess.{c,h} for this, so no code outside that file ever need use WIFEXITED ?
We already have a couple of helper functions, and my recent virFork cleanups got rid of even more clients of WIFEXITED (that is, virCommandRun now defaults to returning sanitized rather than raw exit values). At this point, there are very few reasons for any new code to need to use WIFEXITED; it's mostly limited to existing code (but where my virFork cleanups tripped up on the FreeBSD header bug due to refactoring).
So, should I just ditch this patch? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Fri, Mar 07, 2014 at 09:56:10AM -0700, Eric Blake wrote:
On 03/04/2014 06:59 AM, Eric Blake wrote:
On 03/04/2014 06:47 AM, Daniel P. Berrange wrote:
On Tue, Mar 04, 2014 at 06:28:17AM -0700, Eric Blake wrote:
POSIX requires that <stdlib.h> expose WIFEXITED and friends, but FreeBSD and others fail to comply. We can work around it manually by including <sys/wait.h>, or we can work around it automatically by using gnulib's system-posix module.
* bootstrap.conf (gnulib_modules): Add system-posix.
Signed-off-by: Eric Blake <eblake@redhat.com> ---
I'll wait for a review on this one - we don't use system(), and it feels a little bit odd to list the system() module merely for its side effect of a fixed <stdlib.h>. The alternative is to continue using <sys/wait.h> everywhere that we use WIFEXITED and friends.
Perhaps we should have a helper in util/virprocess.{c,h} for this, so no code outside that file ever need use WIFEXITED ?
We already have a couple of helper functions, and my recent virFork cleanups got rid of even more clients of WIFEXITED (that is, virCommandRun now defaults to returning sanitized rather than raw exit values). At this point, there are very few reasons for any new code to need to use WIFEXITED; it's mostly limited to existing code (but where my virFork cleanups tripped up on the FreeBSD header bug due to refactoring).
So, should I just ditch this patch?
I don't feel strongly either way. ACK if you thing it is worth doing anyway. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 03/07/2014 10:09 AM, Daniel P. Berrange wrote:
On Fri, Mar 07, 2014 at 09:56:10AM -0700, Eric Blake wrote:
On 03/04/2014 06:59 AM, Eric Blake wrote:
On 03/04/2014 06:47 AM, Daniel P. Berrange wrote:
On Tue, Mar 04, 2014 at 06:28:17AM -0700, Eric Blake wrote:
POSIX requires that <stdlib.h> expose WIFEXITED and friends, but FreeBSD and others fail to comply. We can work around it manually by including <sys/wait.h>, or we can work around it automatically by using gnulib's system-posix module.
We already have a couple of helper functions, and my recent virFork cleanups got rid of even more clients of WIFEXITED (that is, virCommandRun now defaults to returning sanitized rather than raw exit values). At this point, there are very few reasons for any new code to need to use WIFEXITED; it's mostly limited to existing code (but where my virFork cleanups tripped up on the FreeBSD header bug due to refactoring).
So, should I just ditch this patch?
I don't feel strongly either way. ACK if you thing it is worth doing anyway.
Roman, any thoughts, since you are the person most likely impacted by this? If we do the gnulib change, then future patches that assume POSIX semantics, and pass testing when written by developers against glibc, won't break the build on BSD; on the other hand, we've made virprocess.h useful enough that most future patches shouldn't be using WIFEXITED directly and therefore shouldn't trip up BSD compilation in the first place. Meanwhile, do you want to file a bug report to the BSD folks to fix their <stdio.h>? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Eric Blake wrote:
On 03/07/2014 10:09 AM, Daniel P. Berrange wrote:
On Fri, Mar 07, 2014 at 09:56:10AM -0700, Eric Blake wrote:
On 03/04/2014 06:59 AM, Eric Blake wrote:
On 03/04/2014 06:47 AM, Daniel P. Berrange wrote:
On Tue, Mar 04, 2014 at 06:28:17AM -0700, Eric Blake wrote:
POSIX requires that <stdlib.h> expose WIFEXITED and friends, but FreeBSD and others fail to comply. We can work around it manually by including <sys/wait.h>, or we can work around it automatically by using gnulib's system-posix module.
We already have a couple of helper functions, and my recent virFork cleanups got rid of even more clients of WIFEXITED (that is, virCommandRun now defaults to returning sanitized rather than raw exit values). At this point, there are very few reasons for any new code to need to use WIFEXITED; it's mostly limited to existing code (but where my virFork cleanups tripped up on the FreeBSD header bug due to refactoring).
So, should I just ditch this patch?
I don't feel strongly either way. ACK if you thing it is worth doing anyway.
Roman, any thoughts, since you are the person most likely impacted by this? If we do the gnulib change, then future patches that assume POSIX semantics, and pass testing when written by developers against glibc, won't break the build on BSD; on the other hand, we've made virprocess.h useful enough that most future patches shouldn't be using WIFEXITED directly and therefore shouldn't trip up BSD compilation in the first place. Meanwhile, do you want to file a bug report to the BSD folks to fix their <stdio.h>?
I think it's worth to include that patch because it costs nothing and helps not to occasionally spot this problem again in future. I'll make a bug report. Thanks, Roman Bogorodskiy
On 03/08/2014 09:57 AM, Roman Bogorodskiy wrote:
Eric Blake wrote:
> POSIX requires that <stdlib.h> expose WIFEXITED and friends, > but FreeBSD and others fail to comply. We can work around it > manually by including <sys/wait.h>, or we can work around it > automatically by using gnulib's system-posix module.
Except system-posix is currently LGPLv3+, which makes it incompatible with libvirt unless gnulib relaxes it.
So, should I just ditch this patch?
I don't feel strongly either way. ACK if you thing it is worth doing anyway.
Roman, any thoughts, since you are the person most likely impacted by this? If we do the gnulib change, then future patches that assume POSIX semantics, and pass testing when written by developers against glibc, won't break the build on BSD; on the other hand, we've made virprocess.h useful enough that most future patches shouldn't be using WIFEXITED directly and therefore shouldn't trip up BSD compilation in the first place. Meanwhile, do you want to file a bug report to the BSD folks to fix their <stdio.h>?
I think it's worth to include that patch because it costs nothing and helps not to occasionally spot this problem again in future.
I've dropped the patch for now (gnulib would have to relax the license before I could use it, and that may take a while).
I'll make a bug report.
Thanks. Looks like it spurred Garrett into investigating it further, as he asked the POSIX folks more about the situation: http://thread.gmane.org/gmane.comp.standards.posix.austin.general/9179 -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Roman Bogorodskiy