Re: [PATCH 1/5] Introduce build_debug_file_name

Gary Benson
2015-06-17T09:47:38Z

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

--