Re: [libvirt] [PATCH 1/3] leasetime support for <dhcp> globally

On 06/22/2017 01:12 PM, Alberto Ruiz wrote: > > > 2017-06-21 17:30 GMT+01:00 Laine Stump <laine(a)laine.org > <mailto:laine@laine.org>>: > > On 06/21/2017 03:27 AM, Peter Krempa wrote: > > On Tue, Jun 20, 2017 at 19:00:43 +0100, aruiz(a)gnome.org <mailto:aruiz@gnome.org> wrote: > >> From: Alberto Ruiz <aruiz(a)gnome.org <mailto:aruiz@gnome.org>> > > > >> + > >> + if (result > UINT32_MAX) { > >> + virReportError(VIR_ERR_XML_ERROR, > >> + _("<leasetime> value cannot be greater than the equivalent of %" PRIo32 " seconds : %" PRId64), > > We don't use gnulib's "PRIxxx" macros anywhere else in libvirt. Better > to "go with the flow" and just use "%d" instead for consistency (or make > a case for changing them elsewhere :-) (Is it possible for you to change the "quotation" character in your email client so that when you inline quote the original in a reply, it has "> " at the beginning of the line rather than " "? Using blanks as the quotation character makes it more likely to confuse who wrote which parts (especially when everything isn't starting in column 1))

> I knew I added this for something: > > This is the error I get when I just use "%d": > > conf/network_conf.c: In function 'virNetworkDHCPLeaseTimeParseXML': > conf/network_conf.c:575:26: error: format '%d' expects argument of type > 'int', but argument 8 has type 'int64_t {aka long int}' [-Werror=format=] > _("<leasetime> value cannot be greater than the > equivalent of %d seconds : %d"), > > If you can think of any alternative that complies with libvirt's code > consistency let me know. Looking through the other struct definitions that are filled in from XML, we don't use any types that directly specify the number of bits. Instead we use int, long, and long long (sometimes with unsigned, sometimes without). On x86_64, long and long long are both 64 bits. I think I would just use "unsigned long" (assuming you agree to not use "-1" as a special value), then use %lu for the formatting string.