[prev in list] [next in list] [prev in thread] [next in thread]
List: git
Subject: Re: [PATCH v3 20/23] try_remove_empty_parents(): don't trash argument contents
From: Jeff King <peff () peff ! net>
Date: 2016-12-31 6:40:53
Message-ID: 20161231064053.prvlw6x6qprzkmw7 () sigill ! intra ! peff ! net
[Download RAW message or body]
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
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic