Message-ID: <alpine.LFD.2.11.1407051529040.15455@eddie.linux-mips.org>
Date:	Sat, 5 Jul 2014 15:56:23 +0100 (BST)
From:	"Maciej W. Rozycki" <macro@...ux-mips.org>
To:	Joe Perches <joe@...ches.com>,
	Grant Likely <grant.likely@...retlab.ca>
cc:	David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] declance: Fix 64-bit compilation warnings

On Wed, 2 Jul 2014, Joe Perches wrote:

> > > > > The kernel vsprintf implementation doesn't prefix
> > > > > pointers with 0x, so you can use 0x%p if you really
> > > > > want that with a leading prefix, but you don't have
> > > > > to use it.
> > > > 
> > > >  It does, when the `#' format modifier is used (go try yourself!).
> > > 
> > > I know it does, but it's incidental.
> > 
> >  Is it?  Someone took the effort to handle it:
> > 
> > 	int default_width = 2 * sizeof(void *) + (spec.flags & SPECIAL ? 2 : 0);
> > 
> > while they could do:
> > 
> > 	int default_width = 2 * sizeof(void *);
> > 
> > 	spec.flags &= ~SPECIAL;
> 
> Grant Likely did that a couple of years ago.
> 
> commit 725fe002d315c2501c110b7245d3eb4f4535f4d6
> Author: Grant Likely <grant.likely@...retlab.ca>
> Date:   Thu May 31 16:26:08 2012 -0700
> 
>     vsprintf: correctly handle width when '#' flag used in %#p format
>     
>     The '%p' output of the kernel's vsprintf() uses spec.field_width to
>     determine how many digits to output based on 2 * sizeof(void*) so that all
>     digits of a pointer are shown.  ie.  a pointer will be output as
>     "001A2B3C" instead of "1A2B3C".  However, if the '#' flag is used in the
>     format (%#p), then the code doesn't take into account the width of the
>     '0x' prefix and will end up outputing "0x1A2B3C" instead of "0x001A2B3C".
>     
>     This patch reworks the "pointer()" format hook to include 2 characters for
>     the '0x' prefix if the '#' flag is included.
> 
> Not sure it ever mattered myself.
> 
> >  Hmm, actually I wonder if GCC maintainers could be persuaded to accept a 
> > `linux_printk' format checker, that would accurately match our semantics 
> > and could handle some of our other extensions too.  There are precedents 
> > already, `cmn_err' and `CFString' (for Solaris and Darwin), so it's not 
> > like a no-no outright.  WDYT?
> > 
> 
> Maybe via a gcc plugin.
> 
> https://gcc.gnu.org/wiki/plugins
> 
> If you're going to write one, it'd be nice to validate all
> the %p<foo> extensions too.

 Well, it would be easier for me to tweak GCC itself, I have never had 
anything to do with plugins.

* Advantages of having it in GCC: everyone will have it, eventually.  
  Disadvantages: they'll have to upgrade the compiler to a version that's 
  recent enough (or backport the change).

* Advantages of having it in a plugin: GCC versions from 4.5 up can be 
  supported, many people won't have to upgrade the compiler.  
  Disadvantages: most likely nobody will use it. ;)

Anyway, I won't rush implementing it, sorry, there's too much other stuff 
I want to do I consider important, though I can place it somewhere down my 
to-do list.

 One question though, does either of you or anybody else know why we're 
inconsistent about this 0x prefixing of virtual addresses vs physical 
addresses?  Specifically %p vs e.g. %pad.  I have fixed (patch just 
posted) and made use of some debug code in the defxx driver and I get 
output like this:

Descriptor block virt = a8000000ce0ec000, phys = 0x00000000ce0ec000
Command Request buffer virt = a8000000ce0ed380, phys = 0x00000000ce0ed380
Command Response buffer virt = a8000000ce0ed580, phys = 0x00000000ce0ed580
Receive buffer block virt = a8000000ce0ed780, phys = 0x00000000ce0ed780
Consumer block virt = a8000000ce0ed780, phys = 0x00000000ce0ed780

For %p the SPECIAL flag is set according to the presence or absence of the 
`#' modifier.  For %pad and friends the flag is forced on.  I find it 
messy.  Can we decide one way or the other?  I have no strong preference 
towards either format, but I think it would be good to stay consistent, 
also in the handling of the modifier.

 Given that (as you've noticed) people want to see that 0x prefix anyway 
perhaps we can add it to %p by default too (and then let it be switched 
off with `#' across all the relevant %p formats)?  Any thoughts?

 Adding LKML for some more feedback perhaps.

  Maciej
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists