From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54586) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bvhcw-0000m5-6W for qemu-devel@nongnu.org; Sun, 16 Oct 2016 05:21:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bvhcs-00029u-Pz for qemu-devel@nongnu.org; Sun, 16 Oct 2016 05:21:06 -0400 Received: from 7.mo179.mail-out.ovh.net ([46.105.61.94]:58540) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1bvhcs-00028L-GH for qemu-devel@nongnu.org; Sun, 16 Oct 2016 05:21:02 -0400 Received: from player792.ha.ovh.net (b7.ovh.net [213.186.33.57]) by mo179.mail-out.ovh.net (Postfix) with ESMTP id ACC74FF987B for ; Sun, 16 Oct 2016 11:20:58 +0200 (CEST) Date: Sun, 16 Oct 2016 11:20:52 +0200 From: Greg Kurz Message-ID: <20161016112052.4942a79e@bahia> In-Reply-To: References: <147648037766.14770.15179642645810274907.stgit@bahia> <20161015000600.1cc5d7a4@bahia> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] trace: fix group name generation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Eric Blake , QEMU Developers , Stefan Hajnoczi On Sat, 15 Oct 2016 13:03:10 +0100 Peter Maydell wrote: > On 14 October 2016 at 23:06, Greg Kurz wrote: > > On Fri, 14 Oct 2016 16:31:01 -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. > > >> > 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