Re: [PATCH 2/2] Trim trailing directory separators from new sysroots.

[Simon Marchi <simark@simark.ca>]

On 2019-01-22 8:03 p.m., John Baldwin wrote:
> The separate debugfile logic assumes that the sysroot does not include
> a trailing separator (all of the sysroot logic in
> find_separate_debug_file requires the next character after the sysroot
> in a object file path to be a directory separator).  However, normal
> filename completion will result in setting a sysroot with a trailing
> directory separator.  If the directory portion of a new sysroot ends
> with a directory separator and is not specifying the root directory,
> trim the trailing separator.  This permits the sysroot logic to work
> the same if a sysroot of "/myroot/" is specified instead of "/myroot".
> 
> Note that the sysroot displayed by 'show sysroot' will reflect the
> trimmed sysroot.

I am not sure about this, it seems fragile to do this one off thing.  The
point where the path is stripped is quite far from the point where it has
an impact, so it's really not obvious why we do it (though this could be
fixed by saying why we do it in the comment).

I think it would be a step in a better direction to have an "is_child_path"
function that returns whether a path is a child of another one.  It would
be responsible for handling the case of the parent potentially having a
trailing directory separator.  The implementation of such function might
get hairy, but the  advantage is that it should be pretty easy to unit
test.

Finally, I think it would make this code more obvious:

      if (canon_dir != NULL
	  && filename_ncmp (canon_dir, gdb_sysroot,
			    strlen (gdb_sysroot)) == 0
	  && IS_DIR_SEPARATOR (canon_dir[strlen (gdb_sysroot)]))

vs

      if (canon_dir != NULL && is_child_patn (gdb_sysroot, canon_dir))

Simon