Re: [PATCH] mini-os: enable compiler check for printk format types

[Samuel Thibault <samuel.thibault@ens-lyon.org>]

Ian Campbell, le Fri 03 Oct 2014 09:55:15 +0100, a écrit :
> On Fri, 2014-10-03 at 09:44 +0100, Thomas Leonard wrote:
> > On 8 August 2014 15:32, Samuel Thibault <samuel.thibault@ens-lyon.org> wrote:
> > > Thomas Leonard, le Fri 08 Aug 2014 15:28:22 +0100, a écrit :
> > >> On 7 August 2014 15:55, Samuel Thibault <samuel.thibault@ens-lyon.org> wrote:
> > >> > Thomas Leonard, le Wed 06 Aug 2014 10:44:00 +0100, a écrit :
> > >> [...]
> > >> >> diff --git a/extras/mini-os/netfront.c b/extras/mini-os/netfront.c
> > >> >> index 44c3995..6f335fe 100644
> > >> >> --- a/extras/mini-os/netfront.c
> > >> >> +++ b/extras/mini-os/netfront.c
> > >> >> @@ -327,8 +327,8 @@ struct netfront_dev *init_netfront(char *_nodename, void (*thenetif_rx)(unsigned
> > >> >>      dev->fd = -1;
> > >> >>  #endif
> > >> >>
> > >> >> -    printk("net TX ring size %d\n", NET_TX_RING_SIZE);
> > >> >> -    printk("net RX ring size %d\n", NET_RX_RING_SIZE);
> > >> >> +    printk("net TX ring size %llu\n", (unsigned long long) NET_TX_RING_SIZE);
> > >> >> +    printk("net RX ring size %llu\n", (unsigned long long) NET_RX_RING_SIZE);
> > >> >
> > >> > lib/printf.c does not actually support %ll yet, it uses %L instead.
> > >>
> > >> Actually, it looks like it does:
> > >>
> > >> if (qualifier == 'l' && *fmt == 'l') {
> > >>   qualifier = 'L';
> > >>   ++fmt;
> > >> }
> > >
> > > Oh, I missed this part of the code, thanks. For this exerpt of code,
> > > long long is way too much, but for things like disk offsets and such,
> > > it'd be preferrable to go for long long.
> > 
> > I see this patch never go applied. To be clear: the above comment was
> > just for information. I agree with using %lu in the patch (as the
> > latest version of the patch does).
> > 
> > So, I think this is ready to be applied:
> > 
> > http://lists.xenproject.org/archives/html/xen-devel/2014-08/msg00998.html
> 
> Samuel, are you ok with the patch after those clarifications?

Yes.

Reviewing it just kept being behind loads of stuff in my TODO list :/

Samuel