[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] trace: fix group name generation
From: |
Greg Kurz |
Subject: |
Re: [Qemu-devel] [PATCH] trace: fix group name generation |
Date: |
Sat, 15 Oct 2016 00:06:00 +0200 |
On Fri, 14 Oct 2016 16:31:01 -0500
Eric Blake <address@hidden> 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 <address@hidden>
> > ---
> > 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?
>
Heh, because the complaint was about the build break :)
> 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.
>
You're right. I'm okay to look for a way to fix the symbol variable size
issue, but I think this patch still makes sense as it makes tracetool
less fragile in case we add a directory with an illegal character to
the QEMU tree... and it fixes annoying build breaks (Paolo also hit
this and asked to revert 80dd5c4918ab in another mail).
Cc'ing Peter...
pgpxunQn9sW_P.pgp
Description: OpenPGP digital signature