[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: |
Sun, 16 Oct 2016 11:20:52 +0200 |
On Sat, 15 Oct 2016 13:03:10 +0100
Peter Maydell <address@hidden> wrote:
> On 14 October 2016 at 23:06, Greg Kurz <address@hidden> wrote:
> > 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.
>
> >> > 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).
>
> I agree with Eric that we should just not be putting
> the build directory name in these variables -- this
> looks like it's simply a bug to me, and we should fix it.
>
As I was saying in the previous mail, I also agree with Eric :)
And it isn't even about the variable size of the debugging info,
but rather about the random names of the generated symbols...
> I think the chances of us adding a directory to the QEMU
> tree itself with a silly name are quite low (not least because
> if you do that then you get a handy build failure to tell
> you not to do that ;-))
>
Fair enough, even if the error is a bit obscure at first sight... and
we already have a silly named directory (9pfs), which would be supported
with a leading '_'... but hopefully, it is not at the top level ;)
> thanks
> -- PMM
Cheers.
--
Greg