Re: [RFA] New substitute-path commands

[Joel Brobecker <brobecker at adacore dot com>]

Hi Daniel,

> > +static char *
> > +xrewrite_source_path (const char *path)
> 
> Why the x?  Is it because it allocates?  I believe we actually use the
> x to indicate replacements for system functionality which have a more
> reliable error behavior; so please don't use it here.

Oops, sorry, misunderstanding on my part. I just removed the x.
I was confused because of xfullpath...

> > +  /* Search for the first occurence of SUBSTITUTE_PATH_FROM.
> > +     No substitution needed of not found.  */
> 
> If not found.

Fixed. Thanks for catching it.

> > +  from_start = strstr (path, substitute_path_from);
> 
> This is the part of the patch I'm not sure about.  I have two
> questions.
> 
> 1.  Should the substitution rule be anchored to the beginning of the
> pathname?
> 
> 2.  Should the end of the rule be anchored to end of string or
> directory separator?
> 
> I think #1 should be yes, and #2 should probably be yes, but I'm less
> certain about it.  What do you think?

  (1): Anchoring means that we replace from the start right, so
       no search within the path, just a strncmp, correct?

       It might be simpler, but on the other hand less flexible.
       I would personally prefer to have it non anchored, but
       it's only a mild preference and I don't know of any
       situation where having the replacement rule anchored
       would cause us any problem in practice.

  (2): I am personally not against this, and it would fit what
       we would do at AdaCore, but I don't see this as a necessary
       constraint that the debugger should check. Again, a mild
       preference.

What do others think? I'm happy to implement whatever the group
thinks is best.

Thanks for the review Daniel,
-- 
Joel