Re: [Qemu-devel] [PATCH] trace: fix group name generation

[Greg Kurz <groug@kaod.org>]

On Tue, 18 Oct 2016 16:16:46 +0100
"Daniel P. Berrange" <berrange@redhat.com> wrote:

> On Fri, Oct 14, 2016 at 04:31:01PM -0500, Eric Blake wrote:
> > On 10/14/2016 04:26 PM, Greg Kurz wrote:  
> > > Since commit "80dd5c4918ab trace: introduce a formal group name for trace
> > > events", tracetool generates C variable names and macro definitions out
> > > of the path to the trace-events-all file.
> > > 
> > > The current code takes care of turning '/' and '-' characters into
> > > underscores so that the resulting names are valid C tokens. This is
> > > enough because these are the only illegal characters that appear in
> > > a relative path within the QEMU source tree.
> > > 
> > > Things are different for out of tree builds where the path may contain
> > > arbitrary character combinations, causing tracetool to generate invalid
> > > names.
> > >   
> >   
> > > This patch ensures that only letters [A-Za-z], digits [0-9] and underscores
> > > are kept. All other characters are turned into underscores. Also, since the
> > > first character of C symbol names cannot be a digit, an underscore is
> > > prepended to the group name.
> > > 
> > > Signed-off-by: Greg Kurz <groug@kaod.org>
> > > ---
> > >  scripts/tracetool.py |    2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/scripts/tracetool.py b/scripts/tracetool.py
> > > index 629b2593c846..b81b834db924 100755
> > > --- a/scripts/tracetool.py
> > > +++ b/scripts/tracetool.py
> > > @@ -70,7 +70,7 @@ def make_group_name(filename):
> > >  
> > >      if dirname == "":
> > >          return "common"
> > > -    return re.sub(r"/|-", "_", dirname)
> > > +    return "_" + re.sub(r"[^\w]", "_", dirname)  
> > 
> > This STILL doesn't solve the complaint that the build is now dependent
> > on the location.  Why can't we STRIP off any prefix prior to the in-tree
> > portion of the naming that we know is sane, instead of munging the
> > prefix but in the process creating source code that generates with
> > different lengths?
> > 
> > Ideally, compiling twice, once in directory 'a', and the second time in
> > directory 'aaaaaaaaaaaaaaaaaaaaaaaaaaaa', should not make a noticeable
> > difference in the final size of the executable due to the difference in
> > lengths of the debugging symbols used to record the longer name of the
> > second directory being encoded into lots of macro names.  
> 
> This is a mistake on my part - the code was supposed to be stripping
> off the build directory prefix, leaving only the relative path to the
> file wrt source directory. The code is simply wrong as is.
> 

I don't know how that can be done without tracetool having knowledge of
what the build directory actually is. My first thought was to rely on
os.getcwd() since tracetool is invoked in the build directory, but I'm
now inclined to implement --group and let the caller (i.e. the makefile)
decide for the group name.

Cc'ing Peter who had some thoughts on the question.

> Regards,
> Daniel

Cheers.

--
Greg