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

[Alex Elder <aelder@sgi.com>]

On Wed, 2011-09-21 at 09:05 +1000, Dave Chinner wrote:
> 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@sgi.com>
> > 
> > ---
> >  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?

I haven't checked right now but I wrote a small thesis
in an e-mail a few months ago about it.  But as I recall
uint64_t was defined based on long on some architectures
and based on long long in others.  And __u64 is based
on long long everywhere in the kernel.  Something like
that.  OK, I went and found it.

http://oss.sgi.com/archives/xfs/2011-07/msg00399.html

The problem lies in the kernel, which defines
__u64 as a an (unsigned long) in ia64, but as
(unsigned long long) in x86_64.  This would be
fine, except that the user space code uses
(unsigned long) as u_int64_t for both architectures
(and that, after all is what PRIu64 is for).

Hence for inodes (and anything else defined as
a __u64) you have to use explicit casts because
PRIu64 won't always work for you.

					-Alex
> 
> 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.



_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs