On Tue, Nov 5, 2013 at 11:41 PM, Joey Ye <joey.ye...@gmail.com> wrote:
> Ping
>
> ChangeLog
>
>       2013-10-27  Vladimir Simonov  <vladimir.simo...@acronis.com>
>
> (include)
>       filename.h (FILENAME_NORMALIZE): New macro.
>       (filename_normalize): New declare.
>
> (libiberty)
>       filename_cmp.c (memmove_left): New function.
>       (filename_normalize): Likewise.
>       getpwd.c (getpwd): Use FILENAME_NORMALIZE.
>
> (libcpp)
>       directives.c (find_file_in_dir): Use FILENAME_NORMALIZE.
>
> Patch is at
> http://gcc.gnu.org/ml/gcc-patches/2013-10/msg02257.html

Your ChangeLog is missing the '*' characters.  See the existing
examples.

I don't see any reason to use the FILENAME_NORMALIZE macro.  Just
always declare a function filename_normalize.  Just don't have it do
anything when not on DOS.

We already have a function in libiberty, lrealpath, that does
something similar to this.  Why not change that to do what you need?
If that would work, it seems preferable.  Then you wouldn't have to
change filenames.h or libcpp.

The comment for filename_normalize needs to explain what the function
actually does.

I don't find the code for filename_normalize particularly easy to
read, but the use of memmove_left appears to turn a linear algorithm
into a quadratic one.  As far as I can tell the goal here is to remove
certain character sequences from the path.  If I am correct, I think
it would be much simpler to write this code more like this (untested):

  p = fn;
  t = fn;
  while (*p != '\0')
    {
      if (!IS_DIR_SEPARATOR (*p))
        *t++ = *p++;
      else if (p[1] == '.' && IS_DIR_SEPARATOR (p[2]))
        p += 2;
      else if (p[1] == '.' && p[2] == '.' && IS_DIR_SEPARATOR (p[3]))
        {
          while (t > fn && ! IS_DIR_SEPARATOR (*t))
            --t;
          if (t > fn)
            ++t;
          p += 3;
        }
      else
        {
          *t++ = '/';
          p++;
        }
    }

Either way I would recommend writing some tests.

Ian

Reply via email to