git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 8/9] remote-curl: store url as a strbuf

From
Jeff King <peff@peff.net>
Date
Sep 28, 2013, 08:35 UTC
Message-ID
<20130928083525.GC2782@sigill.intra.peff.net>
In-Reply-To
<20130928082956.GA22610@sigill.intra.peff.net>

We use a strbuf to generate the string containing the remote URL, but then detach it to a bare pointer. This makes it harder to later manipulate the URL, as we have forgotten the length (and the allocation semantics are not as clear).

Let's instead keep the strbuf around. As a bonus, this eliminates a confusing double-use of the "buf" strbuf in main(). Prior to this, it was used both for constructing the url, and for reading commands from stdin.

The downside is that we have to update each call site to refer to "url.buf" rather than just "url" when they want the C string.

Signed-off-by: Jeff King <peff@peff.net>
---
 remote-curl.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/remote-curl.c b/remote-curl.c
index 7fb092f..5add2cb 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -12,7 +12,8 @@ static struct remote *remote;
 #include "credential.h"
 
 static struct remote *remote;
-static const char *url; /* always ends with a trailing slash */
+/* always ends with a trailing slash */
+static struct strbuf url = STRBUF_INIT;
 
 struct options {
 	int verbosity;
@@ -131,7 +132,8 @@ static struct ref *parse_info_refs(struct discovery *heads)
 			mid = &data[i];
 		if (data[i] == '\n') {
 			if (mid - start != 40)
-				die("%sinfo/refs not valid: is this a git repository?", url);
+				die("%sinfo/refs not valid: is this a git repository?",
+				    url.buf);
 			data[i] = 0;
 			ref_name = mid + 1;
 			ref = xmalloc(sizeof(struct ref) +
@@ -150,7 +152,7 @@ static struct ref *parse_info_refs(struct discovery *heads)
 	}
 
 	ref = alloc_ref("HEAD");
-	if (!http_fetch_ref(url, ref) &&
+	if (!http_fetch_ref(url.buf, ref) &&
 	    !resolve_remote_symref(ref, refs)) {
 		ref->next = refs;
 		refs = ref;
@@ -213,11 +215,11 @@ static struct discovery* discover_refs(const char *service, int for_push)
 		return last;
 	free_discovery(last);
 
-	strbuf_addf(&refs_url, "%sinfo/refs", url);
-	if ((!prefixcmp(url, "http://") || !prefixcmp(url, "https://")) &&
+	strbuf_addf(&refs_url, "%sinfo/refs", url.buf);
+	if ((!prefixcmp(url.buf, "http://") || !prefixcmp(url.buf, "https://")) &&
 	     git_env_bool("GIT_SMART_HTTP", 1)) {
 		maybe_smart = 1;
-		if (!strchr(url, '?'))
+		if (!strchr(url.buf, '?'))
 			strbuf_addch(&refs_url, '?');
 		else
 			strbuf_addch(&refs_url, '&');
@@ -235,13 +237,13 @@ static struct discovery* discover_refs(const char *service, int for_push)
 		break;
 	case HTTP_MISSING_TARGET:
 		show_http_message(&type, &buffer);
-		die("repository '%s' not found", url);
+		die("repository '%s' not found", url.buf);
 	case HTTP_NOAUTH:
 		show_http_message(&type, &buffer);
-		die("Authentication failed for '%s'", url);
+		die("Authentication failed for '%s'", url.buf);
 	default:
 		show_http_message(&type, &buffer);
-		die("unable to access '%s': %s", url, curl_errorstr);
+		die("unable to access '%s': %s", url.buf, curl_errorstr);
 	}
 
 	last= xcalloc(1, sizeof(*last_discovery));
@@ -607,7 +609,7 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads)
 	rpc->out = client.out;
 	strbuf_init(&rpc->result, 0);
 
-	strbuf_addf(&buf, "%s%s", url, svc);
+	strbuf_addf(&buf, "%s%s", url.buf, svc);
 	rpc->service_url = strbuf_detach(&buf, NULL);
 
 	strbuf_addf(&buf, "Content-Type: application/x-%s-request", svc);
@@ -659,7 +661,7 @@ static int fetch_dumb(int nr_heads, struct ref **to_fetch)
 	for (i = 0; i < nr_heads; i++)
 		targets[i] = xstrdup(sha1_to_hex(to_fetch[i]->old_sha1));
 
-	walker = get_http_walker(url);
+	walker = get_http_walker(url.buf);
 	walker->get_all = 1;
 	walker->get_tree = 1;
 	walker->get_history = 1;
@@ -706,7 +708,7 @@ static int fetch_git(struct discovery *heads,
 		depth_arg = strbuf_detach(&buf, NULL);
 		argv[argc++] = depth_arg;
 	}
-	argv[argc++] = url;
+	argv[argc++] = url.buf;
 	argv[argc++] = NULL;
 
 	for (i = 0; i < nr_heads; i++) {
@@ -804,7 +806,7 @@ static int push_dav(int nr_spec, char **specs)
 		argv[argc++] = "--dry-run";
 	if (options.verbosity > 1)
 		argv[argc++] = "--verbose";
-	argv[argc++] = url;
+	argv[argc++] = url.buf;
 	for (i = 0; i < nr_spec; i++)
 		argv[argc++] = specs[i];
 	argv[argc++] = NULL;
@@ -837,7 +839,7 @@ static int push_git(struct discovery *heads, int nr_spec, char **specs)
 	argv_array_push(&args, options.progress ? "--progress" : "--no-progress");
 	for_each_string_list_item(cas_option, &cas_options)
 		argv_array_push(&args, cas_option->string);
-	argv_array_push(&args, url);
+	argv_array_push(&args, url.buf);
 	for (i = 0; i < nr_spec; i++)
 		argv_array_push(&args, specs[i]);
 
@@ -918,14 +920,12 @@ int main(int argc, const char **argv)
 	remote = remote_get(argv[1]);
 
 	if (argc > 2) {
-		end_url_with_slash(&buf, argv[2]);
+		end_url_with_slash(&url, argv[2]);
 	} else {
-		end_url_with_slash(&buf, remote->url[0]);
+		end_url_with_slash(&url, remote->url[0]);
 	}
 
-	url = strbuf_detach(&buf, NULL);
-
-	http_init(remote, url, 0);
+	http_init(remote, url.buf, 0);
 
 	do {
 		if (strbuf_getline(&buf, stdin, '\n') == EOF) {

-- 
1.8.4.rc3.19.g9da5bf6
Previous: Jeff KingNext: Jeff King
Message 16 of 17 in “following http redirects”
  1. 0/9 following http redirectsJeff King, Sep 28, 2013
  2. 1/9 http_get_file: style fixesJeff King, Sep 28, 2013
  3. 2/9 http_request: factor out curlinfo_strbufJeff King, Sep 28, 2013
  4. 3/9 http: refactor options to http_get_*Jeff King, Sep 28, 2013
  5. 4/9 http: hoist credential request out of handle_curl_resultJeff King, Sep 28, 2013
  6. 5/9 http: provide effective url to callersJeff King, Sep 28, 2013
  7. 6/9 http: update base URLs when we see redirectsJeff King, Sep 28, 2013
  8. brian m. carlsonSep 29, 2013
  9. Eric SunshineSep 29, 2013
  10. Jeff KingSep 29, 2013
  11. Eric SunshineSep 29, 2013
  12. Junio C HamanoOct 18, 2013
  13. Jeff KingOct 18, 2013
  14. Junio C HamanoOct 18, 2013
  15. 7/9 remote-curl: make refs_url a strbufJeff King, Sep 28, 2013
  16. 8/9 remote-curl: store url as a strbufJeff King, Sep 28, 2013
  17. 9/9 remote-curl: rewrite base url from info/refs redirectsJeff King, Sep 28, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.