Re: [PATCH v3 20/23] try_remove_empty_parents(): don't trash argument contents
From: Jeff King <hidden>
Date: 2016-12-31 06:41:05
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:
quoted hunk ↗ jump to hunk
@@ -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