On Tue, 2006-04-18 at 09:04 -0400, Daniel Jacobowitz wrote:
> On Tue, Apr 18, 2006 at 02:32:09PM +0200, Frederic RISS wrote:
> > All this file matching seems quite fragile, and the current approach
> > will get it wrong sometimes. Shouldn't the loop in dwarf2_start_subfile
> > be killed in favor of a search using both xfullpath(dirname'/'filename)
> > at the start of buildsym.c::start_subfile?
> 
> The problem with using xfullpath is that it requires the file to exist

I realized that after having sent the patch.

> and already be found; as long as we are consistent, it's not too
> fragile, because we're checking against our own output.

I just made that comment because of the testsuite breakage that ensued
from modifying dirname before the loop... which seemed to imply that the
mechanism was fragile.

> > +
> > +  if (dirname == NULL)
> > +    dirname = comp_dir;
> > +  else
> > +    if (!IS_ABSOLUTE_PATH (dirname) && comp_dir != NULL)
> > +      {
> > +	dirname = concat (comp_dir, SLASH_STRING, dirname, (char *)NULL);
> > +	/* do_cleanups is called after line info procesing */
> > +	make_cleanup (xfree, dirname);
> 
> Comment formatting again (and "processing").  But this is really nasty. 
> Why not free it before we return?  We don't return dirname, and
> start_subfile takes a copy.

I kind of copied it from the end of decode_dwarf_lines where
make_cleanups are used for the same kind of things.
Thanks for your review, I'm attaching an updated patch.

Fred.