oh crap... (re: %p6)

oh crap... (re: %p6)

[David Miller]

I think we'll need to revert all of this or find some fix soon.  GCC
warns about %p with a '#' flag modifier it seems :-(

net/ipv6/ip6_flowlabel.c: In function 'ip6fl_seq_show':
net/ipv6/ip6_flowlabel.c:707: warning: '#' flag used with '%p' printf format

this is with:

davem@sunset:~/src/GIT/net-next-2.6$ gcc --version
gcc (GCC) 4.1.3 20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2)
Copyright (C) 2006 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

davem@sunset:~/src/GIT/net-next-2.6$ 

Re: oh crap... (re: %p6)

[Harvey Harrison]

On Wed, 2008-10-29 at 01:54 -0700, David Miller wrote:
> I think we'll need to revert all of this or find some fix soon.  GCC
> warns about %p with a '#' flag modifier it seems :-(
> 
> net/ipv6/ip6_flowlabel.c: In function 'ip6fl_seq_show':
> net/ipv6/ip6_flowlabel.c:707: warning: '#' flag used with '%p' printf format

The easiest would be to pick a separate modifier for the raw case.  Or
revert the removal of NIP6_SEQFMT, I'll shoot you a patch shortly,
any preference in what modifier should be used?

Harvey

Re: oh crap... (re: %p6)

[Joe Perches]

On Wed, 2008-10-29 at 07:55 -0700, Harvey Harrison wrote:
> On Wed, 2008-10-29 at 01:54 -0700, David Miller wrote:
> > I think we'll need to revert all of this or find some fix soon.  GCC
> > warns about %p with a '#' flag modifier it seems :-(
> > 
> > net/ipv6/ip6_flowlabel.c: In function 'ip6fl_seq_show':
> > net/ipv6/ip6_flowlabel.c:707: warning: '#' flag used with '%p' printf format
> 
> The easiest would be to pick a separate modifier for the raw case.  Or
> revert the removal of NIP6_SEQFMT, I'll shoot you a patch shortly,
> any preference in what modifier should be used?

I'd prefer something visual that represents "no colon".

Maybe %p6-: 

Or perhaps slightly worse to change the colon separated 
default %p6 to %p:6

There are another cases where the v6 address should have
as few leading zeros as possible or where the interior
zeros should be collapsed using :: separators.

Perhaps a good solution is to use the equivalent of
a "display-hint" after the %p6

Maybe use combinations of:

%p6:: use "::" and collapse when 0
%p6-: raw hex
%p6-0 %x not %04x
%p6.4 last u32 as ipv4 dotted decimal

Re: oh crap... (re: %p6)

[Harvey Harrison]

On Wed, 2008-10-29 at 08:39 -0700, Joe Perches wrote:
> On Wed, 2008-10-29 at 07:55 -0700, Harvey Harrison wrote:
> > On Wed, 2008-10-29 at 01:54 -0700, David Miller wrote:
> > > I think we'll need to revert all of this or find some fix soon.  GCC
> > > warns about %p with a '#' flag modifier it seems :-(
> > > 
> > > net/ipv6/ip6_flowlabel.c: In function 'ip6fl_seq_show':
> > > net/ipv6/ip6_flowlabel.c:707: warning: '#' flag used with '%p' printf format
> > 
> > The easiest would be to pick a separate modifier for the raw case.  Or
> > revert the removal of NIP6_SEQFMT, I'll shoot you a patch shortly,
> > any preference in what modifier should be used?
> 
> I'd prefer something visual that represents "no colon".
> 
> Maybe %p6-: 

Putting a modifier after the format specifier seems a little strange to me.  But
I'm not sure if I can think of something that's too much better, currently I
was thinking of:

%pI6, %pi6
	%pI6 would act just like %p6
	%pi6 would act just like %#p6

Eventually %pI4 could be added as well. Which would print out the dot-separated
format.

Either that or add a %px<bytes> that prints out raw hex for the number
of <bytes>, so %p6 would stay and %#p6 would become %px16.  While this
is a bit uglier, it could be useful to others elsewhere in-kernel,
perhaps limited to 16 bytes or so.

Dave, I'll wait for your $0.02 before sending in a patch, but I have
both of the above implemented.

Harvey

Re: oh crap... (re: %p6)

[Joe Perches]

On Wed, 2008-10-29 at 10:24 -0700, Harvey Harrison wrote:
> Putting a modifier after the format specifier seems a little strange to me.

Harvey, please remember the whole %p<foo> concept is based on
modifier after format specifier.

I think %p6 followed by combinations of things:

"::" compressed v6, largest block of consecutive 0's replaced with ::
"-:" no separating colons, use space instead
"-0" no leading 0's
".4" last word as ipv4 dotted decimal
"x1" u8 form
"x2" be16
"x4" be32
"x8" be64

So that you could have:

%p6::		1234:000a::c0a8:0101
%p6::-0		1234:a::c0a8:0101
%p6::-0.4	1234:a::192.168.1.1
%p6-0-:x1	0 1 2 3 4 5 6 7 8 9 a b c d e f
%p6x1		00:01:02:03:04:05:06:07:09:0a:0b:0c:0d:0e:0f

etc.  If acceptable, I'll submit a patch.

Re: oh crap... (re: %p6)

[Harvey Harrison]

On Wed, 2008-10-29 at 11:16 -0700, Joe Perches wrote:
> On Wed, 2008-10-29 at 10:24 -0700, Harvey Harrison wrote:
> > Putting a modifier after the format specifier seems a little strange to me.
> 
> Harvey, please remember the whole %p<foo> concept is based on
> modifier after format specifier.
> 
> I think %p6 followed by combinations of things:
> 
> "::" compressed v6, largest block of consecutive 0's replaced with ::
> "-:" no separating colons, use space instead
> "-0" no leading 0's
> ".4" last word as ipv4 dotted decimal
> "x1" u8 form
> "x2" be16
> "x4" be32
> "x8" be64
> 

Yes, but if you put anything but alphanumerics after the %p, make sure
you handle that in vsnprintf(), so it will be a bit more involved than
my simplistic ones were.

> So that you could have:
> 
> %p6::		1234:000a::c0a8:0101
> %p6::-0		1234:a::c0a8:0101
> %p6::-0.4	1234:a::192.168.1.1
> %p6-0-:x1	0 1 2 3 4 5 6 7 8 9 a b c d e f
> %p6x1		00:01:02:03:04:05:06:07:09:0a:0b:0c:0d:0e:0f
> 
> etc.  If acceptable, I'll submit a patch.
> 

I'll let Dave comment on whether this flexibility is desired.  Mine was
more an attempt to consolidate a very common (and simplisitic) usage.


Cheers,

Harvey

Re: oh crap...

[David Miller]

From: Harvey Harrison <harvey.harrison@gmail.com>
Date: Wed, 29 Oct 2008 11:21:19 -0700

> I'll let Dave comment on whether this flexibility is desired.  Mine was
> more an attempt to consolidate a very common (and simplisitic) usage.

I like Harvey's suggestion the best.

Re: oh crap...

[David Miller]

From: Joe Perches <joe@perches.com>
Date: Wed, 29 Oct 2008 11:16:54 -0700

> On Wed, 2008-10-29 at 10:24 -0700, Harvey Harrison wrote:
> > Putting a modifier after the format specifier seems a little strange to me.
> 
> Harvey, please remember the whole %p<foo> concept is based on
> modifier after format specifier.

I don't think there is any such strict rule, or even that we'd
want to enforce that.

There is nothing wrong with saying something like %pI6 where
the "I" signifies "internet address in 'natural' form" and
"6" is the 'modifier' which you love so much which means
"oh btw, it's ipv6"

Re: oh crap...

[Joe Perches]

On Wed, 2008-10-29 at 11:31 -0700, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> Date: Wed, 29 Oct 2008 11:16:54 -0700
> > On Wed, 2008-10-29 at 10:24 -0700, Harvey Harrison wrote:
> > > Putting a modifier after the format specifier seems a little strange to me.
> > Harvey, please remember the whole %p<foo> concept is based on
> > modifier after format specifier.
> I don't think there is any such strict rule, or even that we'd
> want to enforce that.

Of course there's a strict rule.

"%<flags>p" is a normal format conversion specifier.
It takes a pointer as its argument.
Character(s) immediately after the pointer conversion
specifier "p" may be linux-specific format modifiers.

> There is nothing wrong with saying something like %pI6 where
> the "I" signifies "internet address in 'natural' form" and
> "6" is the 'modifier' which you love so much which means
> "oh btw, it's ipv6"

True enough.  6 and I6 are both modifiers.

I have been doodling with sparse to verify the pointer types.
I just want a simple rule: char after "%<flag>p" specifies pointer type.

Ideally it would be:

4: __be32 *
6: struct in6_addr *
R: struct resource *
M: char *

cheers, Joe

Re: oh crap...

[David Miller]

From: Harvey Harrison <harvey.harrison@gmail.com>
Date: Wed, 29 Oct 2008 10:24:42 -0700

> %pI6, %pi6
> 	%pI6 would act just like %p6
> 	%pi6 would act just like %#p6

I think this form is not a bad idea.

Please send me the patch that does this.

And will you be sending the ipv4 side stuff next?

Thanks.

Re: oh crap...

[Harvey Harrison]

On Wed, 2008-10-29 at 11:28 -0700, David Miller wrote:
> From: Harvey Harrison <harvey.harrison@gmail.com>
> Date: Wed, 29 Oct 2008 10:24:42 -0700
> 
> > %pI6, %pi6
> > 	%pI6 would act just like %p6
> > 	%pi6 would act just like %#p6
> 
> I think this form is not a bad idea.
> 
> Please send me the patch that does this.
> 
> And will you be sending the ipv4 side stuff next?
> 

I can do if you'd like it, I'll sneak %pI4 into the patch fixing up
the warnings I've made and do a conversion of the IPv4 stuff later this
week.

Cheers,

Harvey