Re: [PATCH] builtin-remote: (get_one_entry): use strbuf
From: Jeff King <hidden>
Date: 2016-06-15 22:46:58
On Mon, Jun 22, 2009 at 10:24:19AM +0200, Bert Wesarg wrote:
quoted hunk ↗ jump to hunk
--- a/builtin-remote.c +++ b/builtin-remote.c@@ -1276,17 +1276,17 @@ static int update(int argc, const char **argv) static int get_one_entry(struct remote *remote, void *priv) { struct string_list *list = priv; + struct strbuf url_buf = STRBUF_INIT; + const char *url_str = NULL; const char **url; int i, url_nr; - void **utilp; if (remote->url_nr > 0) { - utilp = &(string_list_append(remote->name, list)->util); - *utilp = xmalloc(strlen(remote->url[0])+strlen(" (fetch)")+1); - strcpy((char *) *utilp, remote->url[0]); - strcat((char *) *utilp, " (fetch)"); - } else - string_list_append(remote->name, list)->util = NULL; + strbuf_addf(&url_buf, "%s (fetch)", remote->url[0]); + url_str = strbuf_detach(&url_buf, NULL); + } + string_list_append(remote->name, list)->util = url_str; +
This causes const warnings due to the 'const' on url_str. One solution
is just s/const char/char/.
However, I think it is actually more readable to do away with url_str
entirely. The original if/else logic was much easier to follow than
realizing that url_str is initialized to NULL, and then never changed.
IOW:
if (remote->url_nr > 0) {
strbuf_addf(&url_buf, "%s (fetch)", remote->url[0]);
string_list_append(remote->name, list)->util =
strbuf_detach(&url_buf, NULL);
}
else
string_list_append(remote->name, list)->util = NULL;
quoted hunk ↗ jump to hunk
@@ -1296,10 +1296,9 @@ static int get_one_entry(struct remote *remote, void *priv) } for (i = 0; i < url_nr; i++) { - utilp = &(string_list_append(remote->name, list)->util); - *utilp = xmalloc(strlen(url[i])+strlen(" (push)")+1); - strcpy((char *) *utilp, url[i]); - strcat((char *) *utilp, " (push)"); + strbuf_addf(&url_buf, "%s (push)", url[i]); + url_str = strbuf_detach(&url_buf, NULL); + string_list_append(remote->name, list)->util = url_str; }
And then this one is re-using url_str for a totally unrelated thing,
but could just be:
string_list_append(remote->name, list)->util =
strbuf_detach(&url_buf, NULL);
But that is somewhat nit-picking. As long as the const-warning goes
away, I will be happy enough.
-Peff