This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] set/unset/show substitute-path commands (take 2)
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