On Sat, Dec 31, 2016 at 04:13:00AM +0100, Michael Haggerty wrote:
> It's bad manners and surprising and therefore error-prone.
Agreed.
I wondered while reading your earlier patch whether the
safe_create_leading_directories() function had the same problem, but it
carefully replaces the NUL it inserts.
> -static void try_remove_empty_parents(char *refname)
> +static void try_remove_empty_parents(const char *refname)
> {
> + struct strbuf buf = STRBUF_INIT;
> char *p, *q;
> int i;
> - p = refname;
> +
> + strbuf_addstr(&buf, refname);
I see here you just make a copy. I think it would be enough to do:
> @@ -2305,10 +2306,11 @@ static void try_remove_empty_parents(char *refname)
> q--;
> if (q == p)
> break;
> - *q = '\0';
> - if (rmdir(git_path("%s", refname)))
> + strbuf_setlen(&buf, q - buf.buf);
> + if (rmdir(git_path("%s", buf.buf)))
> break;
*q = '\0';
r = rmdir(git_path("%s", refname));
*q = '/';
if (r)
break;
or something. But I doubt the single allocation is breaking the bank,
and it has the nice side effect that callers can pass in a const string
(I didn't check yet whether that enables further cleanups).
-Peff