Re: [PATCH 1/5] Introduce build_debug_file_name

[Gary Benson <gbenson at redhat dot com>]

Eli Zaretskii wrote:
> > From: Gary Benson <gbenson@redhat.com>
> > Cc: Cédric Buissart <cedric.buissart@gmail.com>
> > Date: Tue, 16 Jun 2015 10:42:44 +0100
> > 
> > +/* Build the filename of a separate debug file from an arbitrary
> > +   number of components.  Returns an xmalloc'd string that must
> > +   be be freed by the caller.  The final argument of this function
> > +   must be NULL to mark the end the argument list.  */
> > +
> > +static char *
> > +build_debug_file_name (const char *first, ...)
> > +{
> > +  va_list ap;
> > +  const char *arg, *last;
> > +  VEC (char_ptr) *args = NULL;
> > +  struct cleanup *back_to = make_cleanup_free_char_ptr_vec (args);
> > +  int bufsiz = 0;
> > +  char *buf, *tmp;
> > +  int i;
> > +
> > +  va_start (ap, first);
> > +  for (arg = first; arg; arg = va_arg (ap, const char *))
> > +    last = arg;
> > +  va_end (ap);
> > +
> > +  va_start (ap, first);
> > +  for (arg = first; arg; arg = va_arg (ap, const char *))
> > +    {
> > +      if (arg == last)
> > +	tmp = xstrdup (arg);
> > +      else
> > +	{
> > +	  int len;
> > +
> > +	  /* Strip leading separators from subdirectories.  */
> > +	  if (arg != first)
> > +	    {
> > +	      while (*arg != '\0' && IS_DIR_SEPARATOR (*arg))
> > +		arg++;
> > +	    }
> > +
> > +	  /* Strip trailing separators.  */
> > +	  len = strlen (arg);
> > +
> > +	  while (len > 0 && IS_DIR_SEPARATOR (arg[len - 1]))
> > +	    len--;
> 
> Was this logic tested with Windows-style "d:/foo" file names?  E.g.,
> what will happen if the first component is "d:/"?

I don't have access to any Windows machine so I haven't been able to
test this, but I don't think the new code would regress compared to
what it replaces (which doesn't seem to have been written with Windows
in mind at all, e.g. it uses "/" rather than SLASH_STRING and has no
particular handling for drive letters).

For the case you mention nothing would be stripped (the "d" in that
path is !IS_DIR_SEPARATOR) so the filename components would be
concatenated verbatim, just as with the original code.  The resulting
filename may not make sense, but it's not a regression.

I don't believe this series should be blocked unless it breaks
something that actually worked before.

Thanks,
Gary

-- 
http://gbenson.net/