xfs
[Top] [All Lists]

Re: xfsprogs: fix some printf() warnings that show up for ia64 builds

To: Alex Elder <aelder@xxxxxxx>
Subject: Re: xfsprogs: fix some printf() warnings that show up for ia64 builds
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 21 Sep 2011 09:05:46 +1000
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <1316544193.2912.34.camel@doink>
References: <20110814201239.GA26453@xxxxxxxxxxxxx> <20110829083852.GA31515@xxxxxxxxxxxxx> <20110830052226.GM3162@dastard> <20110830085737.GA24793@xxxxxxxxxxxxx> <1316544193.2912.34.camel@doink>
User-agent: Mutt/1.5.21 (2010-09-15)
On Tue, Sep 20, 2011 at 01:43:13PM -0500, Alex Elder wrote:
> This applies on top of Christoph Hellwig's recent "xfs_repair: add
> printf format checking and fix the fallout" patch.  It extends the
> fixes for warnings beyond just xfs_repair and across everything in
> xfsprogs.
> 
> It builds cleanly on ia64 and x86_64, and builds without any
> printf() format-related warnings on i386.
> 
> Signed-off-by: Alex Elder <aelder@xxxxxxx>
> 
> ---
>  io/parent.c              |   28 ++++++++++++++++------------
>  logprint/log_misc.c      |   34 ++++++++++++++++++++--------------
>  logprint/log_print_all.c |   16 ++++++++++------
>  repair/dinode.c          |   20 ++++++++++++--------
>  repair/scan.c            |   14 +++++++++-----
>  5 files changed, 67 insertions(+), 45 deletions(-)
> 
> Index: b/io/parent.c
> ===================================================================
> --- a/io/parent.c
> +++ b/io/parent.c
> @@ -52,12 +52,12 @@ check_parent_entry(xfs_bstat_t *bstatp,
>         if (sts != 0) {
>                 fprintf(stderr,
>                         _("inode-path for inode: %llu is incorrect - path 
> \"%s\" non-existent\n"),
> -                       bstatp->bs_ino, fullpath);
> +                       (unsigned long long) bstatp->bs_ino, fullpath);

Hmmm, didn't Christoph fix these inode number warnings by changing
the format specifier to PRIU64, not by adding casts? bs_ino is
defined as:

        __u64           bs_ino;

So PRIu64 is the right thing to do, isn't it?

Either way would work, but being consistent would be good. ;)

....

> ===================================================================
> --- a/logprint/log_misc.c
> +++ b/logprint/log_misc.c
> @@ -307,12 +307,14 @@ xlog_print_trans_buffer(xfs_caddr_t *ptr
>                          */
>                         memmove(&x, *ptr, sizeof(__be64));
>                         memmove(&y, *ptr+8, sizeof(__be64));
> -                       printf(_("icount: %lld  ifree: %lld  "),
> -                               be64_to_cpu(x), be64_to_cpu(y));
> +                       printf(_("icount: %llu  ifree: %llu  "),
> +                               (unsigned long long) be64_to_cpu(x),
> +                               (unsigned long long) be64_to_cpu(y));

Same for al the be64_to_cpu() functions - their return type is
__u64, too.

>                                         forkname);
> @@ -1374,7 +1376,7 @@ process_lclinode(
>                                                 XFS_DFORK_DSIZE(dip, mp)) {
>                 do_warn(
>         _("local inode %" PRIu64 " data fork is too large (size = %lld, max = 
> %d)\n"),
> -                       lino, be64_to_cpu(dip->di_size),
> +                       lino, (unsigned long long) be64_to_cpu(dip->di_size),

That format specifier is wrong - it is %lld. Should be %llu, or
PRIu64 as previously mentioned without the cast.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>