On Thu, Nov 30, 2023 at 04:10:00PM +0100, Michal Prívozník wrote:
On 11/30/23 16:05, Andrea Bolognani wrote:
> On Thu, Nov 30, 2023 at 09:30:28AM -0500, Andrea Bolognani wrote:
>> On Thu, Nov 30, 2023 at 02:07:55PM +0000, Daniel P. Berrangé wrote:
>>> +++ b/scripts/rpcgen/tests/test_demo.c
>>> @@ -3,6 +3,10 @@
>>> #include <rpc/xdr.h>
>>> #include <stdbool.h>
>>>
>>> +#ifdef __APPLE__
>>> +# define xdr_uint64_t xdr_u_int64_t
>>> +#endif
>>
>> For the long run, I think it would make more sense to have this
>> workaround as part of the generator's output, so that using
>> VIR_TEST_REGENERATE_OUTPUT will produce the same results regardless
>> of whether it's run on Linux or macOS. It would also avoid the need
>> to add a similar workaround somewhere in the library code the day we
>> start needing uint64_t anywhere in our RPC protocol.
>>
>> As a short-term solution, it's fine :)
>
> Never mind, this very obviously doesn't pass muster:
>
> E AssertionError: assert '\nvoid xdr_T...rn TRUE;\n}\n' ==
> '\nvoid xdr_T...rn TRUE;\n}\n'
> E Skipping 9072 identical leading characters in diff, use -v to show
> E - if (!xdr_uint64_t(xdrs, &objp->suh))
> E + if (!xdr_u_int64_t(xdrs, &objp->suh))
> E ? +
> E return FALSE;
> E if (!xdr_bool(xdrs, &objp->sb))
> E return FALSE;...
> E
> E ...Full output truncated (90 lines hidden), use '-vv' to show
>
> My test setup was a bit wonky so I initially failed to catch it.
> Apologies for the noise.
>
> The approach I suggested above would work, I think, but from a very
> quick look at the generator I wasn't able to figure out how it
> decides whether to use xdr_uint64_t or xdr_u_int64_t in the output.
libtirpc on Linux has both:
# grep -e xdr_u_int64_t -e xdr_uint64_t /usr/include/tirpc/rpc/xdr.h
extern bool_t xdr_u_int64_t(XDR *, u_int64_t *);
extern bool_t xdr_uint64_t(XDR *, uint64_t *);
the macos library has only xdr_u_int64_t so we can hack the generator to
use the latter unconditionally.
As Daniel pointed out earlier, portablexdr only has xdr_uint64_t so
that would break our Windows builds.
I think we should just always emit xdr_uint64_t and include the
#ifdef __APPLE__ workaround in the output. That should make things
work everywhere.
--
Andrea Bolognani / Red Hat / Virtualization