Re: [PATCH 1/1] rebase -r: let `label` generate safer labels

[Matt Rogers <mattr94@gmail.com>]

I agree that the code locally was simple enough.

Ultimately I feel that sanitizing and uniqueifying the label should
probably be done closer together/at the same place.  I'm just not
familiar enough with the codebase to know a good place (if any) to move
that to.  Eventually though this would still need to be expanded further
to protect against reserved filenames (e.g. NUL on windows).  Although
the behavior around these (espescially with file extensions like
NUL.txt) become less reliable, and although they are much more unlikely
to be encountered in practice, are still allowed by git as oneliners.


On Tue, Sep 3, 2019 at 3:51 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > If you care deeply about double dashes and leading dashes, how about
> > this instead?
> >
> >               char *from, *to;
> >
> >               for (from = to = label.buf; *from; from++)
> >                       if ((*from & 0x80) || isalnum(*from))
> >                               *(to++) = *from;
> >                       /* avoid leading dash and double-dashes */
> >                       else if (to != label.buf && to[-1] != '-')
> >                               *(to++) = '-';
> >               strbuf_setlen(&label, to - label.buf);
>
> Simple enough and is a good change when judged locally.
>
> It still would cause readers to wonder if label_oid() later append
> '-%d' to end up with double-dash near the end, etc., which made me
> wonder if the resulting code becomes better if sanitization and
> uniquefying are done at the same single place in the other message.



-- 
Matthew Rogers