Hi! A few review comments:
On Wed, 7 Sep 2016 20:19:20 +0300, Andris Pavenis <andris.pave...@iki.fi> wrote:
> This patch fixes handling header.gcc in subdirectories when command line
> option -remap has been
> used.
(I have not yet looked up what that mechanism actually does.) ;-)
> Current version finds header.gcc in directories from GCC include directory
> search path but
> fails to find them in subdirectories due to missing directory separator.
> --- a/libcpp/files.c
> +++ b/libcpp/files.c
> @@ -1672,7 +1672,7 @@ static char *
> remap_filename (cpp_reader *pfile, _cpp_file *file)
> {
> const char *fname, *p;
> - char *new_dir;
> + char *new_dir, *p3;
> cpp_dir *dir;
> size_t index, len;
>
> @@ -1701,9 +1701,15 @@ remap_filename (cpp_reader *pfile, _cpp_file *file)
> return NULL;
>
> len = dir->len + (p - fname + 1);
> - new_dir = XNEWVEC (char, len + 1);
> + new_dir = XNEWVEC (char, len + 2);
> + p3 = new_dir + dir->len;
> memcpy (new_dir, dir->name, dir->len);
> - memcpy (new_dir + dir->len, fname, p - fname + 1);
> + if (dir->len && !IS_DIR_SEPARATOR(dir->name[dir->len - 1]))
> + {
> + *p3++ = '/';
> + len++;
> + }
> + memcpy (p3, fname, p - fname + 1);
> new_dir[len] = '\0';
>
> dir = make_cpp_dir (pfile, new_dir, dir->sysp);
(I have not yet reviewed the logic of your change itself.) Wouldn't it
be simpler to just unconditionally add a directory separator here?
Is it OK to assume that a directory separator always is "/"? (Think DOS,
Windows etc. But maybe there's some "translation layer" beneath this --
I don't know.)
Can you provide some test cases? (Ah, I now see you got some "Test
script to reproduce problem" attached to <https://gcc.gnu.org/PR71681> --
this should be turned into a regression test for the GCC testsuite.)
It is good practice to assign a GCC PR to yourself if you're working on
it, and it also helps to post to the PR a comment with a link to the
mailing list archive for patch submissions, etc.
Grüße
Thomas
signature.asc
Description: PGP signature
