Re: [Libguestfs] [PATCH 1/5] macosx: Add required third parameter for xdrproc_t callbacks

Daniel P. Berrange
2015-02-09T13:02:49Z

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 :|