Re: [PATCH] tools lib traceevent: Add support for IP address formats

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Thu, Dec 18, 2014 at 11:27:15AM -0500, Steven Rostedt escreveu:
> On Thu, 18 Dec 2014 12:52:55 -0300
> Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> > Em Thu, Dec 18, 2014 at 08:10:43AM -0700, David Ahern escreveu:
> > > Adds helper for following kernel formats:
> > >   %pi4 print an IPv4 address with leading zeros
> > >   %pI4 print an IPv4 address without leading zeros
> > >   %pi6 print an IPv6 address without colons
> > >   %pI6 print an IPv6 address with colons
> > >   %pI6c print an IPv6 address with colons
> > >   %pISpc print an IP address from a sockaddr

> > > Allows these formats to be used in tracepoints.

> > > Quite a bit of this is adapted from code in lib/vsprintf.c.

> > Can't we try as much as possible use that code directly? Something like
 
> Until we have a true shared library, please no. I'm still taking code
> from here and adding it manually to trace-cmd. And also sending patches
> to here from what I add to trace-cmd.
 
> I do not want this directly linked to the kernel code. Note, the kernel
> internals have no guaranteed API, where this does.

If using it directly is frowned upon, at least try to reduce the
differences when copying code, be it by keeping function ordering in the
source code, using the same file name and generally making it easier to
see if changes ocurred on the original file over time.

> > we do for lib/rbtree.c in tools/perf/, or like I did recently with
> > tools/lib/util/find_next_bit.c, i.e. retain at least this kind of
> > sharing:

> > diff -u tools/lib/util/find_next_bit.c  lib/find_next_bit.c
 
> Unless we convert code to trace_seq, I don't think this works. My
> patches to move seq_buf.c int lib/ have made it into the kernel.
> Perhaps we can copy the seq_buf.c code into tools/lib/traceevent, but
> that would take some work.

Well, I thought that what that code was doing was just taking a string,
looking for some specific %LETTER stuff, acting on it and returning the
formatted result, i.e. a vsprintf function, thus I ended up shortcutting
too fast to the conclusion that it was possible, my bad :-)

If take the time to look at the specifics I'll propose something more
concrete, meanwhile, are you ok with David's changes? May I process his
patch?

- Arnaldo