Re: [PATCH] libxl: virtio: Fix build error for 32-bit platforms

[Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>]

On 15/12/2022 3:11 pm, Jan Beulich wrote:
> On 15.12.2022 14:48, Anthony PERARD wrote:
>> On Thu, Dec 15, 2022 at 07:01:40PM +0530, Viresh Kumar wrote:
>>> The field 'base' in 'struct libxl_device_virtio' is defined as uint64,
>>> while we are printing it with '%lu', which is 32bit only 32-bit
>>> platforms. And so generates a error like:
>>>
>>>   libxl_internal.h:4388:51: error: format '%lu' expects argument of type 
>>> 'long
>>>   unsigned int', but argument 3 has type 'uint64_t' {aka 'long long unsigned
>>>   int'} [-Werror=format=]
>>>
>>> Fix the same by using PRIx64 instead.
>>>
>>> Now that the base name is available in hexadecimal format, prefix it
>>> with '0x' as well, which strtoul() also depends upon since base passed
>>> is 0.
>>>
>>> Fixes: 43ba5202e2ee ("libxl: add support for generic virtio device")
>>> Signed-off-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
>>> ---
>>> Couldn't test on 32-bit platforms yet, but works fine for 64 bit one.
>>>
>>>  tools/libs/light/libxl_virtio.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tools/libs/light/libxl_virtio.c 
>>> b/tools/libs/light/libxl_virtio.c
>>> index 6a38def2faf5..2217bda8a253 100644
>>> --- a/tools/libs/light/libxl_virtio.c
>>> +++ b/tools/libs/light/libxl_virtio.c
>>> @@ -45,12 +45,12 @@ static int libxl__set_xenstore_virtio(libxl__gc *gc, 
>>> uint32_t domid,
>>>      const char *transport = 
>>> libxl_virtio_transport_to_string(virtio->transport);
>>>  
>>>      flexarray_append_pair(back, "irq", GCSPRINTF("%u", virtio->irq));
>>> -    flexarray_append_pair(back, "base", GCSPRINTF("%lu", virtio->base));
>>> +    flexarray_append_pair(back, "base", GCSPRINTF("0x%"PRIx64, 
>>> virtio->base));
>> There is also PRIu64 that exist, which would be perfect to replace %u.
>> Could we use that instead?
>>
>> I'd rather not have to think about which base is used for numbers in
>> xenstore. I can't find any hexadecimal numbers in xenstore for a simple
>> guest at the moment, so probably best to avoid adding one. And using
>> hexadecimal isn't needed to fix the build.
> Otoh an address formatted in decimal is pretty unusable to any human
> (who might be inspecting xenstore for whatever reasons).

A consumer of xenstore has to cope with all bases anyway.  Anything that
doesn't is broken.

Keys ought to be expressed in the logical base for a human to read, and
hex for base is the right one in this case.

~Andrew