From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8321 invoked by alias); 24 Jul 2006 20:25:51 -0000 Received: (qmail 8313 invoked by uid 22791); 24 Jul 2006 20:25:51 -0000 X-Spam-Check-By: sourceware.org Received: from nevyn.them.org (HELO nevyn.them.org) (66.93.172.17) by sourceware.org (qpsmtpd/0.31.1) with ESMTP; Mon, 24 Jul 2006 20:25:48 +0000 Received: from drow by nevyn.them.org with local (Exim 4.54) id 1G56zp-0004Lh-9B; Mon, 24 Jul 2006 16:25:45 -0400 Date: Mon, 24 Jul 2006 20:25:00 -0000 From: Daniel Jacobowitz To: Joel Brobecker Cc: gdb-patches@sources.redhat.com Subject: Re: [RFA] set/unset/show substitute-path commands (take 2) Message-ID: <20060724202545.GD15759@nevyn.them.org> Mail-Followup-To: Joel Brobecker , gdb-patches@sources.redhat.com References: <20060715054902.GD1393@adacore.com> <20060719181516.GO15273@adacore.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20060719181516.GO15273@adacore.com> User-Agent: Mutt/1.5.11+cvs20060403 X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2006-07/txt/msg00351.txt.bz2 On Wed, Jul 19, 2006 at 11:15:16AM -0700, Joel Brobecker wrote: > I adjusted the testcase according to the modifications in the code. > I also tried to add a test that would verify that the substitution > correctly takes place, but as expected, I'm having some trouble > doing that. Here is what I did: > > * gdb.base/subst.exp > * gdb.base/subst.c > * gdb.base/subst.dir/subst.c > > The version of subst.c in subst.dir is a modified version of subst.c. > I built the subst executable using gdb.base/subst.c, as usual. And then > tried to add a substitution rule as follow: > > (gdb) set substitute-path '${srcdir}/${subdir}' '${srcdir}/${subdir}/subst.dir' > > But the problem is that ${srcdir} is not string equal to what compiler > put in the debugging information. As far as I can tell from the testcase > output, ${srcdir} is something like ../../gdb/testsuite. I don't see how > to reliably get the source path in a way that would be identical to what > the compiler sets. > > Perhaps this is telling us that, instead of just FILENAME_CMP, we should > use something more sophisticated like xfullpath to check for path equality. > But I'm very reluctant to even suggest that, because performance would > probably become horrendous on large applications where lots and lots of > files are used. Let's not go towards xfullpath for this please. The whole problem is that we don't want to rely on build-time paths if told not to; we shouldn't be affected by what's on the disk now. The best way to handle this is to ask the compiler. Try loading the binary and running "info source"; it will print out the compilation directory. > /* Quick way out if we already know its full name */ > + > if (*fullname) > { > + { > + /* The user may have requested that source paths be rewritten > + according to substitution rules he provided. If a substitution > + rule applies to this path, then apply it. */ > + char *rewritten_fullname = rewrite_source_path (*fullname); > + > + if (rewritten_fullname != NULL) > + { > + xfree (*fullname); > + *fullname = rewritten_fullname; > + } > + } > + > result = open (*fullname, OPEN_MODE); > if (result >= 0) > return result; You don't need the extra { } here. > +/* If the last character of PATH is a directory separator, then strip it. */ > + > +static void > +strip_trailing_directory_separator (char *path) > +{ > + const int last = strlen (path) - 1; > + > + if (last < 0) > + return; /* No stripping is needed if PATH is the empty string. */ > + > + if (IS_DIR_SEPARATOR (path[last])) > + path[last] = '\0'; > +} Is this going to get you in trouble if PATH is "/" ? I can imagine that happening. > +/* A convenience function to push ARGV in the cleanup queue. */ > + > +static void > +make_cleanup_argv (char **argv) > +{ > + make_cleanup ((make_cleanup_ftype *) freeargv, argv); > +} Casting function types is a good way to get in trouble. In this case, there's an answer handy: make_cleanup_freeargv. Otherwise looks great! -- Daniel Jacobowitz CodeSourcery