# [PATCH] archive: simplify refname handling 5 messages from 2012-05-18 to 2012-05-18. Participants: René Scharfe, Jeff King, Carlos Martín Nieto, Junio C Hamano. Thread: https://gitlist.dev/t/30550 ## René Scharfe, 2012-05-18 05:15 Subject: [PATCH] archive: simplify refname handling Message-ID: <4FB5DAE5.6020307@lsrfire.ath.cx> URL: https://gitlist.dev/e/4FB5DAE5.6020307%40lsrfire.ath.cx ``` There is no need to build a copy of the relevant part of the string just to make sure we have a NUL-terminated string. We can simply pass the length of the interesting part to dwim_ref() instead. Signed-off-by: Rene Scharfe --- archive.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/archive.c b/archive.c index cd083ea..ecc1ff0 100644 --- a/archive.c +++ b/archive.c @@ -254,18 +254,11 @@ static void parse_treeish_arg(const char **argv, /* Remotes are only allowed to fetch actual refs */ if (remote) { char *ref = NULL; - const char *refname, *colon = NULL; - - colon = strchr(name, ':'); - if (colon) - refname = xstrndup(name, colon - name); - else - refname = name; - - if (!dwim_ref(refname, strlen(refname), sha1, &ref)) - die("no such ref: %s", refname); - if (refname != name) - free((void *)refname); + const char *colon = strchr(name, ':'); + size_t refnamelen = colon ? colon - name : strlen(name); + + if (!dwim_ref(name, refnamelen, sha1, &ref)) + die("no such ref: %.*s", refnamelen, name); free(ref); } -- 1.7.10.2 ``` ## Jeff King, 2012-05-18 15:59 Subject: Re: [PATCH] archive: simplify refname handling Message-ID: <20120518155903.GA21264@sigill.intra.peff.net> URL: https://gitlist.dev/e/20120518155903.GA21264%40sigill.intra.peff.net In-Reply-To: <4FB5DAE5.6020307@lsrfire.ath.cx> ``` [+cc Carlos, as this is his code] On Fri, May 18, 2012 at 07:15:17AM +0200, René Scharfe wrote: > There is no need to build a copy of the relevant part of the string just > to make sure we have a NUL-terminated string. We can simply pass the > length of the interesting part to dwim_ref() instead. > > Signed-off-by: Rene Scharfe > --- > archive.c | 17 +++++------------ > 1 file changed, 5 insertions(+), 12 deletions(-) > > diff --git a/archive.c b/archive.c > index cd083ea..ecc1ff0 100644 > --- a/archive.c > +++ b/archive.c > @@ -254,18 +254,11 @@ static void parse_treeish_arg(const char **argv, > /* Remotes are only allowed to fetch actual refs */ > if (remote) { > char *ref = NULL; > - const char *refname, *colon = NULL; > - > - colon = strchr(name, ':'); > - if (colon) > - refname = xstrndup(name, colon - name); > - else > - refname = name; > - > - if (!dwim_ref(refname, strlen(refname), sha1, &ref)) > - die("no such ref: %s", refname); > - if (refname != name) > - free((void *)refname); > + const char *colon = strchr(name, ':'); > + size_t refnamelen = colon ? colon - name : strlen(name); > + > + if (!dwim_ref(name, refnamelen, sha1, &ref)) > + die("no such ref: %.*s", refnamelen, name); > free(ref); Looks obviously correct to me. -Peff ``` ## Carlos Martín Nieto, 2012-05-18 16:44 Subject: Re: [PATCH] archive: simplify refname handling Message-ID: <1337359483.3083.2.camel@centaur.lab.cmartin.tk> URL: https://gitlist.dev/e/1337359483.3083.2.camel%40centaur.lab.cmartin.tk In-Reply-To: <20120518155903.GA21264@sigill.intra.peff.net> ``` On Fri, 2012-05-18 at 11:59 -0400, Jeff King wrote: > [+cc Carlos, as this is his code] > > On Fri, May 18, 2012 at 07:15:17AM +0200, René Scharfe wrote: > > > There is no need to build a copy of the relevant part of the string just > > to make sure we have a NUL-terminated string. We can simply pass the > > length of the interesting part to dwim_ref() instead. > > > > Signed-off-by: Rene Scharfe > > --- > > archive.c | 17 +++++------------ > > 1 file changed, 5 insertions(+), 12 deletions(-) > > > > diff --git a/archive.c b/archive.c > > index cd083ea..ecc1ff0 100644 > > --- a/archive.c > > +++ b/archive.c > > @@ -254,18 +254,11 @@ static void parse_treeish_arg(const char **argv, > > /* Remotes are only allowed to fetch actual refs */ > > if (remote) { > > char *ref = NULL; > > - const char *refname, *colon = NULL; > > - > > - colon = strchr(name, ':'); > > - if (colon) > > - refname = xstrndup(name, colon - name); > > - else > > - refname = name; > > - > > - if (!dwim_ref(refname, strlen(refname), sha1, &ref)) > > - die("no such ref: %s", refname); > > - if (refname != name) > > - free((void *)refname); > > + const char *colon = strchr(name, ':'); > > + size_t refnamelen = colon ? colon - name : strlen(name); > > + > > + if (!dwim_ref(name, refnamelen, sha1, &ref)) > > + die("no such ref: %.*s", refnamelen, name); > > free(ref); > > Looks obviously correct to me. Yep, that's clearly a much better way of doing it. cmn ``` ## Junio C Hamano, 2012-05-18 18:22 Subject: Re: [PATCH] archive: simplify refname handling Message-ID: URL: https://gitlist.dev/e/xmqq1umh4aeh.fsf%40junio.mtv.corp.google.com In-Reply-To: <4FB5DAE5.6020307@lsrfire.ath.cx> ``` Looks good to me. Thanks. ``` ## Junio C Hamano, 2012-05-18 18:27 Subject: Re: [PATCH] archive: simplify refname handling Message-ID: URL: https://gitlist.dev/e/xmqqwr492vl7.fsf%40junio.mtv.corp.google.com In-Reply-To: <4FB5DAE5.6020307@lsrfire.ath.cx> ``` René Scharfe writes: > + const char *colon = strchr(name, ':'); > + size_t refnamelen = colon ? colon - name : strlen(name); > + > + if (!dwim_ref(name, refnamelen, sha1, &ref)) > + die("no such ref: %.*s", refnamelen, name); > free(ref); > } Both dwim_ref()'s second parameter the vararg that corresponds to the length part of "%.*s" in die(), expect "int", not "size_t", so I needed this to get it compile. archive.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/archive.c b/archive.c index 081ecb5..1e7156d 100644 --- a/archive.c +++ b/archive.c @@ -261,7 +261,7 @@ static void parse_treeish_arg(const char **argv, if (remote) { char *ref = NULL; const char *colon = strchr(name, ':'); - size_t refnamelen = colon ? colon - name : strlen(name); + int refnamelen = colon ? colon - name : strlen(name); if (!dwim_ref(name, refnamelen, sha1, &ref)) die("no such ref: %.*s", refnamelen, name); ```