1. Patchew
  2. Libvirt
  3. View series

[libvirt PATCH] scripts/rpcgen: fix 64 unsigned int test on macOS

Daniel P. Berrangé posted 1 patch 4 months, 4 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20231130092415.98449-1-berrange@redhat.com
There is a newer version of this series
scripts/rpcgen/tests/demo.c | 4 ++++
1 file changed, 4 insertions(+)
[libvirt PATCH] scripts/rpcgen: fix 64 unsigned int test on macOS
Posted by Daniel P. Berrangé 4 months, 4 weeks ago 
macOS XDR library is an oddball using xdr_u_int64_t instead of
xdr_uint64_t which everyone else has.

The code generator already does the right thing, but the test
program previously generated with the Linux rpcgen program
does not compile on macOS due to this.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 scripts/rpcgen/tests/demo.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/scripts/rpcgen/tests/demo.c b/scripts/rpcgen/tests/demo.c
index 182ed448f0..56a50239dc 100644
--- a/scripts/rpcgen/tests/demo.c
+++ b/scripts/rpcgen/tests/demo.c
@@ -1,4 +1,8 @@
 
+#ifdef __APPLE__
+# define xdr_uint64_t xdr_u_int64_t
+#endif
+
 void xdr_TestStruct_clear(TestStruct *objp)
 {
     xdr_free((xdrproc_t)xdr_TestStruct, (char *)objp);
-- 
2.41.0
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [libvirt PATCH] scripts/rpcgen: fix 64 unsigned int test on macOS
Posted by Andrea Bolognani 4 months, 4 weeks ago 
On Thu, Nov 30, 2023 at 09:24:15AM +0000, Daniel P. Berrangé wrote:
> macOS XDR library is an oddball using xdr_u_int64_t instead of
> xdr_uint64_t which everyone else has.
>
> The code generator already does the right thing, but the test
> program previously generated with the Linux rpcgen program
> does not compile on macOS due to this.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  scripts/rpcgen/tests/demo.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/scripts/rpcgen/tests/demo.c b/scripts/rpcgen/tests/demo.c
> index 182ed448f0..56a50239dc 100644
> --- a/scripts/rpcgen/tests/demo.c
> +++ b/scripts/rpcgen/tests/demo.c
> @@ -1,4 +1,8 @@
>
> +#ifdef __APPLE__
> +# define xdr_uint64_t xdr_u_int64_t
> +#endif

This makes the compilation error go away, but I'm not convinced it's
the right fix.

IIUC demo.{c,h} are generated from demo.x, so wouldn't this be
overwritten the next time we regenerate them?

Also both Linux and macOS have xdr_u_int64_t, and we already seem to
use the u_ variant for other things (u_short, u_int), so couldn't we
just use xdr_u_int64_t everywhere and avoid the conditional?

-- 
Andrea Bolognani / Red Hat / Virtualization
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [libvirt PATCH] scripts/rpcgen: fix 64 unsigned int test on macOS
Posted by Daniel P. Berrangé 4 months, 4 weeks ago 
On Thu, Nov 30, 2023 at 05:17:09AM -0500, Andrea Bolognani wrote:
> On Thu, Nov 30, 2023 at 09:24:15AM +0000, Daniel P. Berrangé wrote:
> > macOS XDR library is an oddball using xdr_u_int64_t instead of
> > xdr_uint64_t which everyone else has.
> >
> > The code generator already does the right thing, but the test
> > program previously generated with the Linux rpcgen program
> > does not compile on macOS due to this.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  scripts/rpcgen/tests/demo.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/scripts/rpcgen/tests/demo.c b/scripts/rpcgen/tests/demo.c
> > index 182ed448f0..56a50239dc 100644
> > --- a/scripts/rpcgen/tests/demo.c
> > +++ b/scripts/rpcgen/tests/demo.c
> > @@ -1,4 +1,8 @@
> >
> > +#ifdef __APPLE__
> > +# define xdr_uint64_t xdr_u_int64_t
> > +#endif
> 
> This makes the compilation error go away, but I'm not convinced it's
> the right fix.
> 
> IIUC demo.{c,h} are generated from demo.x, so wouldn't this be
> overwritten the next time we regenerate them?

No, the demo.{c,h} are reference implementations from the original
rpcgen, which we'll never change.

The demo.x is processed by our own RPC generator code, and then we
check that both the original demo.c, and our own generated version
produce the same data on the wire.

> Also both Linux and macOS have xdr_u_int64_t, and we already seem to
> use the u_ variant for other things (u_short, u_int), so couldn't we
> just use xdr_u_int64_t everywhere and avoid the conditional?

IIRC, that would then break Windows

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [libvirt PATCH] scripts/rpcgen: fix 64 unsigned int test on macOS
Posted by Andrea Bolognani 4 months, 4 weeks ago 
On Thu, Nov 30, 2023 at 11:40:17AM +0000, Daniel P. Berrangé wrote:
> On Thu, Nov 30, 2023 at 05:17:09AM -0500, Andrea Bolognani wrote:
> > On Thu, Nov 30, 2023 at 09:24:15AM +0000, Daniel P. Berrangé wrote:
> > > +++ b/scripts/rpcgen/tests/demo.c
> > > +#ifdef __APPLE__
> > > +# define xdr_uint64_t xdr_u_int64_t
> > > +#endif
> >
> > This makes the compilation error go away, but I'm not convinced it's
> > the right fix.
> >
> > IIUC demo.{c,h} are generated from demo.x, so wouldn't this be
> > overwritten the next time we regenerate them?
>
> No, the demo.{c,h} are reference implementations from the original
> rpcgen, which we'll never change.
>
> The demo.x is processed by our own RPC generator code, and then we
> check that both the original demo.c, and our own generated version
> produce the same data on the wire.

Your description doesn't seem to line up with what I'm seeing here:

  $ git status
  On branch rpcgen
  nothing to commit, working tree clean

  $ echo "/* TEST TEST TEST */" >>scripts/rpcgen/tests/demo.c

  $ git status
  On branch rpcgen
  Changes not staged for commit:
    (use "git add <file>..." to update what will be committed)
    (use "git restore <file>..." to discard changes in working directory)
  	modified:   scripts/rpcgen/tests/demo.c

  no changes added to commit (use "git add" and/or "git commit -a")

  $ VIR_TEST_REGENERATE_OUTPUT=1 meson test -C build
  ...

  $ git status
  On branch rpcgen
  nothing to commit, working tree clean

> > Also both Linux and macOS have xdr_u_int64_t, and we already seem to
> > use the u_ variant for other things (u_short, u_int), so couldn't we
> > just use xdr_u_int64_t everywhere and avoid the conditional?
>
> IIRC, that would then break Windows

You're right, portablexdr doesn't seem to have xdr_u_int64_t either.

-- 
Andrea Bolognani / Red Hat / Virtualization
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [libvirt PATCH] scripts/rpcgen: fix 64 unsigned int test on macOS
Posted by Daniel P. Berrangé 4 months, 4 weeks ago 
On Thu, Nov 30, 2023 at 09:00:02AM -0500, Andrea Bolognani wrote:
> On Thu, Nov 30, 2023 at 11:40:17AM +0000, Daniel P. Berrangé wrote:
> > On Thu, Nov 30, 2023 at 05:17:09AM -0500, Andrea Bolognani wrote:
> > > On Thu, Nov 30, 2023 at 09:24:15AM +0000, Daniel P. Berrangé wrote:
> > > > +++ b/scripts/rpcgen/tests/demo.c
> > > > +#ifdef __APPLE__
> > > > +# define xdr_uint64_t xdr_u_int64_t
> > > > +#endif
> > >
> > > This makes the compilation error go away, but I'm not convinced it's
> > > the right fix.
> > >
> > > IIUC demo.{c,h} are generated from demo.x, so wouldn't this be
> > > overwritten the next time we regenerate them?
> >
> > No, the demo.{c,h} are reference implementations from the original
> > rpcgen, which we'll never change.
> >
> > The demo.x is processed by our own RPC generator code, and then we
> > check that both the original demo.c, and our own generated version
> > produce the same data on the wire.
> 
> Your description doesn't seem to line up with what I'm seeing here:
> 
>   $ git status
>   On branch rpcgen
>   nothing to commit, working tree clean
> 
>   $ echo "/* TEST TEST TEST */" >>scripts/rpcgen/tests/demo.c
> 
>   $ git status
>   On branch rpcgen
>   Changes not staged for commit:
>     (use "git add <file>..." to update what will be committed)
>     (use "git restore <file>..." to discard changes in working directory)
>   	modified:   scripts/rpcgen/tests/demo.c
> 
>   no changes added to commit (use "git add" and/or "git commit -a")
> 
>   $ VIR_TEST_REGENERATE_OUTPUT=1 meson test -C build
>   ...
> 
>   $ git status
>   On branch rpcgen
>   nothing to commit, working tree clean

Oh I forgot, test_generator.py uses the demo.c file, in addition to
test_demo.c


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.