Margaret Lewicka
2015-Feb-12 17:28 UTC
[Libguestfs] [PATCH 1/3] macosx: Includes/defines for byteswap operations
--- src/inspect-apps.c | 13 ++++++++++++- src/inspect-fs-windows.c | 6 ++++++ src/journal.c | 5 +++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/inspect-apps.c b/src/inspect-apps.c index 20cf00a..8fbae9c 100644 --- a/src/inspect-apps.c +++ b/src/inspect-apps.c @@ -35,11 +35,22 @@ #include <sys/endian.h> #endif -/* be32toh is usually a macro definend in <endian.h>, but it might be +/* be32toh is usually a macro defined in <endian.h>, but it might be * a function in some system so check both, and if neither is defined * then define be32toh for RHEL 5. */ #if !defined(HAVE_BE32TOH) && !defined(be32toh) + +#if defined __APPLE__ && defined __MACH__ +/* Define/include necessary items on MacOS X */ +#include <machine/endian.h> +#define __BIG_ENDIAN BIG_ENDIAN +#define __LITTLE_ENDIAN LITTLE_ENDIAN +#define __BYTE_ORDER BYTE_ORDER +#include <libkern/OSByteOrder.h> +#define __bswap_32 OSSwapConstInt32 +#endif /* __APPLE__ */ + #if __BYTE_ORDER == __LITTLE_ENDIAN #define be32toh(x) __bswap_32 (x) #else diff --git a/src/inspect-fs-windows.c b/src/inspect-fs-windows.c index 682157a..421a5b1 100644 --- a/src/inspect-fs-windows.c +++ b/src/inspect-fs-windows.c @@ -36,6 +36,12 @@ #include <sys/endian.h> #endif +#if defined __APPLE__ && defined __MACH__ +#include <libkern/OSByteOrder.h> +#define le32toh(x) OSSwapLittleToHostInt32(x) +#define le64toh(x) OSSwapLittleToHostInt64(x) +#endif + #include <pcre.h> #include "c-ctype.h" diff --git a/src/journal.c b/src/journal.c index 1070067..c563b7f 100644 --- a/src/journal.c +++ b/src/journal.c @@ -35,6 +35,11 @@ #include <sys/endian.h> #endif +#if defined __APPLE__ && defined __MACH__ +#include <libkern/OSByteOrder.h> +#define be64toh(x) OSSwapBigToHostInt64(x) +#endif + #include "full-read.h" #include "guestfs.h" -- 1.9.3
Margaret Lewicka
2015-Feb-12 17:28 UTC
[Libguestfs] [PATCH 2/3] builder: Check HAVE_POSIX_FADVISE before using it
--- builder/pxzcat-c.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builder/pxzcat-c.c b/builder/pxzcat-c.c index 0bbd296..dec9cc2 100644 --- a/builder/pxzcat-c.c +++ b/builder/pxzcat-c.c @@ -214,8 +214,10 @@ pxzcat (value filenamev, value outputfilev, unsigned nr_threads) unix_error (err, (char *) "ftruncate", outputfilev); } +#if defined HAVE_POSIX_FADVISE /* Tell the kernel we won't read the output file. */ ignore_value (posix_fadvise (fd, 0, 0, POSIX_FADV_RANDOM|POSIX_FADV_DONTNEED)); +#endif /* Iterate over blocks. */ iter_blocks (idx, nr_threads, filenamev, fd, outputfilev, ofd); -- 1.9.3
Margaret Lewicka
2015-Feb-12 17:28 UTC
[Libguestfs] [PATCH 3/3] lib: Add third, zero parameter to xdrproc_t
As advised by Daniel P. Berrange, the third parameter can be passed on all platforms rather than specifically Mac. Quoting a libvirt commit rationale after Daniel: 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: typedef 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. --- src/proto.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/proto.c b/src/proto.c index 92ae84d..e229abb 100644 --- a/src/proto.c +++ b/src/proto.c @@ -252,7 +252,7 @@ guestfs___send (guestfs_h *g, int proc_nr, * have no parameters. */ if (xdrp) { - if (!(*xdrp) (&xdr, args)) { + if (!(*xdrp) (&xdr, args, 0)) { error (g, _("dispatch failed to marshal args")); return -1; } @@ -681,7 +681,7 @@ guestfs___recv (guestfs_h *g, const char *fn, return -1; } } else { - if (xdrp && ret && !xdrp (&xdr, ret)) { + if (xdrp && ret && !xdrp (&xdr, ret, 0)) { error (g, "%s: failed to parse reply", fn); xdr_destroy (&xdr); return -1; -- 1.9.3
Pino Toscano
2015-Feb-12 18:28 UTC
Re: [Libguestfs] [PATCH 1/3] macosx: Includes/defines for byteswap operations
On Thursday 12 February 2015 17:28:46 Margaret Lewicka wrote:> --- > src/inspect-apps.c | 13 ++++++++++++- > src/inspect-fs-windows.c | 6 ++++++ > src/journal.c | 5 +++++ > 3 files changed, 23 insertions(+), 1 deletion(-) > > diff --git a/src/inspect-apps.c b/src/inspect-apps.c > index 20cf00a..8fbae9c 100644 > --- a/src/inspect-apps.c > +++ b/src/inspect-apps.c > @@ -35,11 +35,22 @@ > #include <sys/endian.h> > #endif > > -/* be32toh is usually a macro definend in <endian.h>, but it might be > +/* be32toh is usually a macro defined in <endian.h>, but it might be > * a function in some system so check both, and if neither is defined > * then define be32toh for RHEL 5. > */ > #if !defined(HAVE_BE32TOH) && !defined(be32toh) > + > +#if defined __APPLE__ && defined __MACH__ > +/* Define/include necessary items on MacOS X */ > +#include <machine/endian.h> > +#define __BIG_ENDIAN BIG_ENDIAN > +#define __LITTLE_ENDIAN LITTLE_ENDIAN > +#define __BYTE_ORDER BYTE_ORDER > +#include <libkern/OSByteOrder.h> > +#define __bswap_32 OSSwapConstInt32 > +#endif /* __APPLE__ */ > + > #if __BYTE_ORDER == __LITTLE_ENDIAN > #define be32toh(x) __bswap_32 (x) > #else > diff --git a/src/inspect-fs-windows.c b/src/inspect-fs-windows.c > index 682157a..421a5b1 100644 > --- a/src/inspect-fs-windows.c > +++ b/src/inspect-fs-windows.c > @@ -36,6 +36,12 @@ > #include <sys/endian.h> > #endif > > +#if defined __APPLE__ && defined __MACH__ > +#include <libkern/OSByteOrder.h> > +#define le32toh(x) OSSwapLittleToHostInt32(x) > +#define le64toh(x) OSSwapLittleToHostInt64(x) > +#endif > + > #include <pcre.h> > > #include "c-ctype.h" > diff --git a/src/journal.c b/src/journal.c > index 1070067..c563b7f 100644 > --- a/src/journal.c > +++ b/src/journal.c > @@ -35,6 +35,11 @@ > #include <sys/endian.h> > #endif > > +#if defined __APPLE__ && defined __MACH__ > +#include <libkern/OSByteOrder.h> > +#define be64toh(x) OSSwapBigToHostInt64(x) > +#endif > + > #include "full-read.h" > > #include "guestfs.h"This code clearly need some refactoring, before adding more copy/pasted parts to it. I'd say to do this in two steps: a) moving the definitions of the current be32toh/etc to a new src/guestfs-byteswap.h (internal), making use of it b) add the proper definitions needed on Mac OS X Thanks, -- Pino Toscano
Pino Toscano
2015-Feb-12 18:30 UTC
Re: [Libguestfs] [PATCH 2/3] builder: Check HAVE_POSIX_FADVISE before using it
On Thursday 12 February 2015 17:28:47 Margaret Lewicka wrote:> --- > builder/pxzcat-c.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/builder/pxzcat-c.c b/builder/pxzcat-c.c > index 0bbd296..dec9cc2 100644 > --- a/builder/pxzcat-c.c > +++ b/builder/pxzcat-c.c > @@ -214,8 +214,10 @@ pxzcat (value filenamev, value outputfilev, unsigned nr_threads) > unix_error (err, (char *) "ftruncate", outputfilev); > } > > +#if defined HAVE_POSIX_FADVISE > /* Tell the kernel we won't read the output file. */ > ignore_value (posix_fadvise (fd, 0, 0, POSIX_FADV_RANDOM|POSIX_FADV_DONTNEED)); > +#endif > > /* Iterate over blocks. */ > iter_blocks (idx, nr_threads, filenamev, fd, outputfilev, ofd); >LGTM. -- Pino Toscano
Margaret Lewicka
2015-Feb-12 18:37 UTC
Re: [Libguestfs] [PATCH 1/3] macosx: Includes/defines for byteswap operations
On 12 February 2015 at 18:28, Pino Toscano <ptoscano@redhat.com> wrote: [much byteswapping]> This code clearly need some refactoring, before adding more copy/pasted > parts to it. I'd say to do this in two steps: > a) moving the definitions of the current be32toh/etc to a new > src/guestfs-byteswap.h (internal), making use of it > b) add the proper definitions needed on Mac OS XHeh. Yet again, I should have gone with the first hunch, but refactoring existing code seemed, um, preposterous for a newbie. Thanks for the suggestion. Is it generally preferred to post updated versions of the same patch (or rather new approaches to patching the same code) with an In-Reply-To: to previous patch, or in a new thread? -- M.
Richard W.M. Jones
2015-Feb-12 20:28 UTC
Re: [Libguestfs] [PATCH 2/3] builder: Check HAVE_POSIX_FADVISE before using it
On Thu, Feb 12, 2015 at 05:28:47PM +0000, Margaret Lewicka wrote:> --- > builder/pxzcat-c.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/builder/pxzcat-c.c b/builder/pxzcat-c.c > index 0bbd296..dec9cc2 100644 > --- a/builder/pxzcat-c.c > +++ b/builder/pxzcat-c.c > @@ -214,8 +214,10 @@ pxzcat (value filenamev, value outputfilev, unsigned nr_threads) > unix_error (err, (char *) "ftruncate", outputfilev); > } > > +#if defined HAVE_POSIX_FADVISE > /* Tell the kernel we won't read the output file. */ > ignore_value (posix_fadvise (fd, 0, 0, POSIX_FADV_RANDOM|POSIX_FADV_DONTNEED)); > +#endif > > /* Iterate over blocks. */ > iter_blocks (idx, nr_threads, filenamev, fd, outputfilev, ofd); > -- > 1.9.3Pushed. Thanks, Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
Richard W.M. Jones
2015-Feb-12 20:33 UTC
Re: [Libguestfs] [PATCH 3/3] lib: Add third, zero parameter to xdrproc_t
On Thu, Feb 12, 2015 at 05:28:48PM +0000, Margaret Lewicka wrote:> As advised by Daniel P. Berrange, the third parameter can be passed on all > platforms rather than specifically Mac. > > Quoting a libvirt commit rationale after Daniel: > > 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: > > typedef 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. > --- > src/proto.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/proto.c b/src/proto.c > index 92ae84d..e229abb 100644 > --- a/src/proto.c > +++ b/src/proto.c > @@ -252,7 +252,7 @@ guestfs___send (guestfs_h *g, int proc_nr, > * have no parameters. > */ > if (xdrp) { > - if (!(*xdrp) (&xdr, args)) { > + if (!(*xdrp) (&xdr, args, 0)) { > error (g, _("dispatch failed to marshal args")); > return -1; > } > @@ -681,7 +681,7 @@ guestfs___recv (guestfs_h *g, const char *fn, > return -1; > } > } else { > - if (xdrp && ret && !xdrp (&xdr, ret)) { > + if (xdrp && ret && !xdrp (&xdr, ret, 0)) { > error (g, "%s: failed to parse reply", fn); > xdr_destroy (&xdr); > return -1; > -- > 1.9.3This works fine for me, so pushed. Thanks, Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
Richard W.M. Jones
2015-Feb-12 20:35 UTC
Re: [Libguestfs] [PATCH 1/3] macosx: Includes/defines for byteswap operations
As Pino said this could probably do with a bit of refactoring to clean it up, but I've committed it anyway because it's a step towards fixing OS X builds. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
Reasonably Related Threads
- [PATCH 1/5] macosx: Add required third parameter for xdrproc_t callbacks
- Re: [PATCH 1/5] macosx: Add required third parameter for xdrproc_t callbacks
- [PATCH 1/3] macosx: Includes/defines for byteswap operations
- Re: Patchable build problems on OS X 10.10
- [PATCH 1/2] gobject: Use generator_built macro to ensure generated files are rebuilt properly.