Margaret Lewicka
2015-Feb-09 11:06 UTC
[Libguestfs] [PATCH 1/5] macosx: Add required third parameter for xdrproc_t callbacks
>From Apple's xdr.h:"If your code invokes an xdrproc_t callback, it must be modified to pass a third parameter, which may simply be zero." --- src/proto.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/proto.c b/src/proto.c index 92ae84d..57f4882 100644 --- a/src/proto.c +++ b/src/proto.c @@ -252,7 +252,12 @@ guestfs___send (guestfs_h *g, int proc_nr, * have no parameters. */ if (xdrp) { +#if !(defined __APPLE__ && defined __MACH__) if (!(*xdrp) (&xdr, args)) { +#else + /* Mac OS X's implementation of xdrproc_t requires a third parameter */ + if (!(*xdrp) (&xdr, args, 0)) { +#endif error (g, _("dispatch failed to marshal args")); return -1; } @@ -681,7 +686,12 @@ guestfs___recv (guestfs_h *g, const char *fn, return -1; } } else { +#if !(defined __APPLE__ && defined __MACH__) if (xdrp && ret && !xdrp (&xdr, ret)) { +#else + /* Mac OS X's implementation of xdrproc_t requires a third parameter */ + if (xdrp && ret && !xdrp (&xdr, ret, 0)) { +#endif error (g, "%s: failed to parse reply", fn); xdr_destroy (&xdr); return -1; -- 1.9.3
Margaret Lewicka
2015-Feb-09 11:06 UTC
[Libguestfs] [PATCH 2/5] macosx: Add definition of program_name for gnulib
gnulib's error.c requires program_name to be externally defined for !_LIBC systems. This defines program_name for Darwin only. --- configure.ac | 3 +++ src/Makefile.am | 6 ++++++ src/program_name.c | 4 ++++ 3 files changed, 13 insertions(+) create mode 100644 src/program_name.c diff --git a/configure.ac b/configure.ac index 37850a3..a2fb99e 100644 --- a/configure.ac +++ b/configure.ac @@ -581,6 +581,9 @@ fi AC_MSG_RESULT([$DISTRO]) AC_SUBST([DISTRO]) +dnl Check if we're building on MacOS X. +AM_CONDITIONAL([DARWIN], [test "x$(uname)" = "xDarwin"]) + dnl Add extra packages to the appliance. AC_ARG_WITH([extra-packages], [AS_HELP_STRING([--with-extra-packages="pkg1 pkg2 ..."], diff --git a/src/Makefile.am b/src/Makefile.am index 2496887..1c36035 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -131,6 +131,12 @@ libguestfs_la_SOURCES = \ tmpdirs.c \ libguestfs.syms +if DARWIN +# gnulib requires program_name to be defined on non-glibc systems, in +# particular MacOS X. +libguestfs_la_SOURCES += program_name.c +endif + libguestfs_la_CPPFLAGS = \ -DGUESTFS_DEFAULT_PATH='"$(libdir)/guestfs"' \ -DGUESTFS_WARN_DEPRECATED=1 \ diff --git a/src/program_name.c b/src/program_name.c new file mode 100644 index 0000000..e1b2e19 --- /dev/null +++ b/src/program_name.c @@ -0,0 +1,4 @@ +/* gnulib requires char *program_name to be defined for non-glibc systems + * (like MacOS X). + */ +const char *program_name = "libguestfs"; -- 1.9.3
Margaret Lewicka
2015-Feb-09 11:06 UTC
[Libguestfs] [PATCH 3/5] java: Turn off doclint to prevent errors on JDK 8
On JDK 8, doclint is enabled by default with strict validation of HTML tags, which causes the build to fail. See http://blog.joda.org/2014/02/turning-off-doclint-in-jdk-8-javadoc.html --- java/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/Makefile.am b/java/Makefile.am index fa5b45c..a76a9d1 100644 --- a/java/Makefile.am +++ b/java/Makefile.am @@ -68,7 +68,7 @@ clean-local: if HAVE_JAVA JAVAC_FLAGS = $(EXTRA_JAVAC_FLAGS) -encoding utf-8 -JAVADOC_FLAGS = -encoding utf-8 +JAVADOC_FLAGS = -encoding utf-8 -Xdoclint:none # Java source. -- 1.9.3
Margaret Lewicka
2015-Feb-09 11:06 UTC
[Libguestfs] [PATCH 4/5] macosx/bsd: Use getprogname() where available
--- configure.ac | 1 + src/guestfs-internal-frontend.h | 2 ++ 2 files changed, 3 insertions(+) diff --git a/configure.ac b/configure.ac index a2fb99e..0f7f97e 100644 --- a/configure.ac +++ b/configure.ac @@ -315,6 +315,7 @@ AC_CHECK_FUNCS([\ be32toh \ fsync \ futimens \ + getprogname \ getxattr \ htonl \ htons \ diff --git a/src/guestfs-internal-frontend.h b/src/guestfs-internal-frontend.h index 2b9070b..d496655 100644 --- a/src/guestfs-internal-frontend.h +++ b/src/guestfs-internal-frontend.h @@ -133,6 +133,8 @@ extern void guestfs___cleanup_pclose (void *ptr); */ #if HAVE_DECL_PROGRAM_INVOCATION_SHORT_NAME == 1 # define guestfs___program_name program_invocation_short_name +#elif HAVE_GETPROGNAME +# define guestfs___program_name getprogname() #else # define guestfs___program_name "libguestfs" #endif -- 1.9.3
Margaret Lewicka
2015-Feb-09 11:06 UTC
[Libguestfs] [PATCH 5/5] macosx/bsd: Alternatives for linux-specific commands
* Workaround for linux-specific fuser -v * Workaround for linux-specific fusermount --- fuse/guestunmount.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/fuse/guestunmount.c b/fuse/guestunmount.c index 3df481b..2190ba0 100644 --- a/fuse/guestunmount.c +++ b/fuse/guestunmount.c @@ -257,7 +257,12 @@ do_fusermount (const char *mountpoint, char **error_rtn) /* We have to parse error messages from fusermount, so ... */ setenv ("LC_ALL", "C", 1); +#ifdef __linux__ execlp ("fusermount", "fusermount", "-u", mountpoint, NULL); +#else + /* use umount where fusermount is not available */ + execlp ("umount", "umount", mountpoint, NULL); +#endif perror ("exec"); _exit (EXIT_FAILURE); } @@ -334,7 +339,19 @@ do_fuser (const char *mountpoint) } if (pid == 0) { /* Child - run /sbin/fuser. */ +#ifdef __linux__ execlp ("/sbin/fuser", "fuser", "-v", "-m", mountpoint, NULL); +#else + /* BSD and Mac OS X versions of fuser do not have the -v option */ + const char *cmd_prefix = "/bin/ps -p \"$(fuser -c "; + const char *cmd_suffix = " 2>/dev/null)\" -o user,pid,comm 2>/dev/null"; + char *cmd = malloc (strlen(cmd_prefix) + strlen(mountpoint) + strlen(cmd_suffix) + 1); + if (cmd) { + sprintf (cmd, "%s%s%s", cmd_prefix, mountpoint, cmd_suffix); + execlp ("/bin/sh", "sh", "-c", cmd, NULL); + free (cmd); + } +#endif _exit (EXIT_FAILURE); } -- 1.9.3
Pino Toscano
2015-Feb-09 12:39 UTC
Re: [Libguestfs] [PATCH 1/5] macosx: Add required third parameter for xdrproc_t callbacks
On Monday 09 February 2015 11:06:15 Margaret Lewicka wrote:> >From Apple's xdr.h: > "If your code invokes an xdrproc_t callback, it must be modified to pass > a third parameter, which may simply be zero." > --- > src/proto.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/src/proto.c b/src/proto.c > index 92ae84d..57f4882 100644 > --- a/src/proto.c > +++ b/src/proto.c > @@ -252,7 +252,12 @@ guestfs___send (guestfs_h *g, int proc_nr, > * have no parameters. > */ > if (xdrp) { > +#if !(defined __APPLE__ && defined __MACH__) > if (!(*xdrp) (&xdr, args)) { > +#else > + /* Mac OS X's implementation of xdrproc_t requires a third parameter */ > + if (!(*xdrp) (&xdr, args, 0)) { > +#endif > error (g, _("dispatch failed to marshal args")); > return -1; > } > @@ -681,7 +686,12 @@ guestfs___recv (guestfs_h *g, const char *fn, > return -1; > } > } else { > +#if !(defined __APPLE__ && defined __MACH__) > if (xdrp && ret && !xdrp (&xdr, ret)) { > +#else > + /* Mac OS X's implementation of xdrproc_t requires a third parameter */ > + if (xdrp && ret && !xdrp (&xdr, ret, 0)) { > +#endif > error (g, "%s: failed to parse reply", fn); > xdr_destroy (&xdr); > return -1;This should rather be a configure check, instead of hardcoding how a libc behaves. Also, what does the third parameter represent? -- Pino Toscano
Daniel P. Berrange
2015-Feb-09 13:02 UTC
Re: [Libguestfs] [PATCH 1/5] macosx: Add required third parameter for xdrproc_t callbacks
On Mon, Feb 09, 2015 at 01:39:55PM +0100, Pino Toscano wrote:> On Monday 09 February 2015 11:06:15 Margaret Lewicka wrote: > > >From Apple's xdr.h: > > "If your code invokes an xdrproc_t callback, it must be modified to pass > > a third parameter, which may simply be zero." > > --- > > src/proto.c | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/src/proto.c b/src/proto.c > > index 92ae84d..57f4882 100644 > > --- a/src/proto.c > > +++ b/src/proto.c > > @@ -252,7 +252,12 @@ guestfs___send (guestfs_h *g, int proc_nr, > > * have no parameters. > > */ > > if (xdrp) { > > +#if !(defined __APPLE__ && defined __MACH__) > > if (!(*xdrp) (&xdr, args)) { > > +#else > > + /* Mac OS X's implementation of xdrproc_t requires a third parameter */ > > + if (!(*xdrp) (&xdr, args, 0)) { > > +#endif > > error (g, _("dispatch failed to marshal args")); > > return -1; > > } > > @@ -681,7 +686,12 @@ guestfs___recv (guestfs_h *g, const char *fn, > > return -1; > > } > > } else { > > +#if !(defined __APPLE__ && defined __MACH__) > > if (xdrp && ret && !xdrp (&xdr, ret)) { > > +#else > > + /* Mac OS X's implementation of xdrproc_t requires a third parameter */ > > + if (xdrp && ret && !xdrp (&xdr, ret, 0)) { > > +#endif > > error (g, "%s: failed to parse reply", fn); > > xdr_destroy (&xdr); > > return -1; > > This should rather be a configure check, instead of hardcoding how a > libc behaves.Actually you can just unconditionally pass the 3rd parameter on all platforms. See this commit in libvirt for rationale: commit 9fa3a8ab6fd82ad2f5a14b490696085061418718 Author: Doug Goldstein <cardoe@cardoe.com> Date: Wed Oct 30 11:22:58 2013 -0500 MacOS: Handle changes to xdrproc_t definition With Mac OS X 10.9, xdrproc_t is no longer defined as: typedef bool_t (*xdrproc_t)(XDR *, ...); but instead as: typdef bool_t (*xdrproc_t)(XDR *, void *, unsigned int); For reference, Linux systems typically define it as: typedef bool_t (*xdrproc_t)(XDR *, void *, ...); The rationale explained in the header is that using a vararg is incorrect and has a potential to change the ABI slightly do to compiler optimizations taken and the undefined behavior. They decided to specify the exact number of parameters and for compatibility with old code decided to make the signature require 3 arguments. The third argument is ignored for cases that its not used and its recommended to supply a 0. 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 :|
Daniel P. Berrange
2015-Feb-09 13:07 UTC
Re: [Libguestfs] [PATCH 5/5] macosx/bsd: Alternatives for linux-specific commands
On Mon, Feb 09, 2015 at 11:06:19AM +0000, Margaret Lewicka wrote:> * Workaround for linux-specific fuser -v > * Workaround for linux-specific fusermount > --- > fuse/guestunmount.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/fuse/guestunmount.c b/fuse/guestunmount.c > index 3df481b..2190ba0 100644 > --- a/fuse/guestunmount.c > +++ b/fuse/guestunmount.c > @@ -257,7 +257,12 @@ do_fusermount (const char *mountpoint, char **error_rtn) > /* We have to parse error messages from fusermount, so ... */ > setenv ("LC_ALL", "C", 1); > > +#ifdef __linux__ > execlp ("fusermount", "fusermount", "-u", mountpoint, NULL); > +#else > + /* use umount where fusermount is not available */ > + execlp ("umount", "umount", mountpoint, NULL); > +#endif > perror ("exec"); > _exit (EXIT_FAILURE); > } > @@ -334,7 +339,19 @@ do_fuser (const char *mountpoint) > } > > if (pid == 0) { /* Child - run /sbin/fuser. */ > +#ifdef __linux__ > execlp ("/sbin/fuser", "fuser", "-v", "-m", mountpoint, NULL); > +#else > + /* BSD and Mac OS X versions of fuser do not have the -v option */ > + const char *cmd_prefix = "/bin/ps -p \"$(fuser -c "; > + const char *cmd_suffix = " 2>/dev/null)\" -o user,pid,comm 2>/dev/null"; > + char *cmd = malloc (strlen(cmd_prefix) + strlen(mountpoint) + strlen(cmd_suffix) + 1); > + if (cmd) { > + sprintf (cmd, "%s%s%s", cmd_prefix, mountpoint, cmd_suffix); > + execlp ("/bin/sh", "sh", "-c", cmd, NULL);This is vulnerable to shell meta characters in the user supplied "mountpoint" string. I dont know if it is a security exploit in the context of libguestfs alone, but it could cause problems when considered apps using libguestfs. Any use of shell should really be avoided when dealing with user supplied input. 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 :|
Daniel P. Berrange
2015-Feb-09 13:10 UTC
Re: [Libguestfs] [PATCH 2/5] macosx: Add definition of program_name for gnulib
On Mon, Feb 09, 2015 at 11:06:16AM +0000, Margaret Lewicka wrote:> gnulib's error.c requires program_name to be externally defined > for !_LIBC systems. This defines program_name for Darwin only. > --- > configure.ac | 3 +++ > src/Makefile.am | 6 ++++++ > src/program_name.c | 4 ++++ > 3 files changed, 13 insertions(+) > create mode 100644 src/program_name.csrc/guestfs-internal-frontend.h has a #define to replace use of 'program_name' with something that is defined, so this should not be required unless the header includes are not right somewhere. 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 :|
Pino Toscano
2015-Feb-09 13:27 UTC
Re: [Libguestfs] [PATCH 3/5] java: Turn off doclint to prevent errors on JDK 8
On Monday 09 February 2015 11:06:17 Margaret Lewicka wrote:> On JDK 8, doclint is enabled by default with strict validation of HTML > tags, which causes the build to fail. See > http://blog.joda.org/2014/02/turning-off-doclint-in-jdk-8-javadoc.html > --- > java/Makefile.am | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/java/Makefile.am b/java/Makefile.am > index fa5b45c..a76a9d1 100644 > --- a/java/Makefile.am > +++ b/java/Makefile.am > @@ -68,7 +68,7 @@ clean-local: > if HAVE_JAVA > > JAVAC_FLAGS = $(EXTRA_JAVAC_FLAGS) -encoding utf-8 > -JAVADOC_FLAGS = -encoding utf-8 > +JAVADOC_FLAGS = -encoding utf-8 -Xdoclint:noneI'd say it would be much better to fix the issues doclint issues, if possible, rather than disabling all of them. -- Pino Toscano
Richard W.M. Jones
2015-Feb-11 13:48 UTC
Re: [Libguestfs] [PATCH 3/5] java: Turn off doclint to prevent errors on JDK 8
On Mon, Feb 09, 2015 at 11:06:17AM +0000, Margaret Lewicka wrote:> On JDK 8, doclint is enabled by default with strict validation of HTML > tags, which causes the build to fail. See > http://blog.joda.org/2014/02/turning-off-doclint-in-jdk-8-javadoc.html > --- > java/Makefile.am | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/java/Makefile.am b/java/Makefile.am > index fa5b45c..a76a9d1 100644 > --- a/java/Makefile.am > +++ b/java/Makefile.am > @@ -68,7 +68,7 @@ clean-local: > if HAVE_JAVA > > JAVAC_FLAGS = $(EXTRA_JAVAC_FLAGS) -encoding utf-8 > -JAVADOC_FLAGS = -encoding utf-8 > +JAVADOC_FLAGS = -encoding utf-8 -Xdoclint:none > > # Java source. >As Pino says it's probably better to fix these. However I accepted this patch and have pushed it upstream. Thanks, Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
Richard W.M. Jones
2015-Feb-11 13:52 UTC
Re: [Libguestfs] [PATCH 4/5] macosx/bsd: Use getprogname() where available
On Mon, Feb 09, 2015 at 11:06:18AM +0000, Margaret Lewicka wrote:> --- > configure.ac | 1 + > src/guestfs-internal-frontend.h | 2 ++ > 2 files changed, 3 insertions(+) > > diff --git a/configure.ac b/configure.ac > index a2fb99e..0f7f97e 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -315,6 +315,7 @@ AC_CHECK_FUNCS([\ > be32toh \ > fsync \ > futimens \ > + getprogname \ > getxattr \ > htonl \ > htons \ > diff --git a/src/guestfs-internal-frontend.h b/src/guestfs-internal-frontend.h > index 2b9070b..d496655 100644 > --- a/src/guestfs-internal-frontend.h > +++ b/src/guestfs-internal-frontend.h > @@ -133,6 +133,8 @@ extern void guestfs___cleanup_pclose (void *ptr); > */ > #if HAVE_DECL_PROGRAM_INVOCATION_SHORT_NAME == 1 > # define guestfs___program_name program_invocation_short_name > +#elif HAVE_GETPROGNAME > +# define guestfs___program_name getprogname() > #else > # define guestfs___program_name "libguestfs" > #endif > --Thanks, ACKed and pushed. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Apparently Analagous Threads
- Re: [PATCH 2/5] macosx: Add definition of program_name for gnulib
- Re: [PATCH 2/5] macosx: Add definition of program_name for gnulib
- Re: [PATCH 2/5] macosx: Add definition of program_name for gnulib
- [PATCH 1/5] macosx: Add required third parameter for xdrproc_t callbacks
- Re: [PATCH] gnulib: Define argv[0] as program_name for error.c on Darwin