Re: [PATCH v8] format-patch: allow a non-integral version numbers

Eric Sunshine [off-list ref] writes:

This new example raises the question about what happens if the
argument to --reroll-count contains characters which don't belong in
pathnames. For instance, what happens if `--reroll-count=1/2` is
specified? Most likely, it will fail trying to write the
"v1/2-whatever.patch" file to a nonexistent directory named "v1".

quoted

diff --git a/log-tree.c b/log-tree.c

@@ -369,8 +369,8 @@ void fmt_output_subject(struct strbuf *filename,
+       if (info->reroll_count)
+               strbuf_addf(filename, "v%s-", info->reroll_count);
        strbuf_addf(filename, "%04d-%s", nr, subject);

To protect against that problem, you may need to call
format_sanitized_subject() manually after formatting "v%s-". (I'm just
looking at this code for the first time, so I could be hopelessly
wrong. There may be a better way to fix it.)

This kind of discovery of what I and others missed is why I love to
see reviews on this list ;-)

Yes, slash is of course very problematic, but what we've been doing
to the patch filenames was to ensure that they will be free of $IFS
whitespaces and shell glob special characters as well, and we should
treat the "reroll count" just like the other end-user controlled
input, i.e. the title of the patch, and sanitize it the same way.

So I am pretty sure format_sanitized_subject() is the right way to
go.