read_branches_file: simplify string handling

This function does a lot of manual string handling, and has some unnecessary limits. This patch cleans up a number of things: 1. Drop the arbitrary 1000-byte limit on the size of the remote name (we do not have such a limit in any of the other remote-reading mechanisms). 2. Replace fgets into a fixed-size buffer with a strbuf, eliminating any limits on the length of the URL. 3. Replace manual whitespace handling with strbuf_trim (since we now have a strbuf). This also gets rid of a call to strcpy, and the confusing reuse of the "p" pointer for multiple purposes. 4. We currently build up the refspecs over multiple strbuf calls. We do this to handle the fact that the URL "frag" may not be present. But rather than have multiple conditionals, let's just default "frag" to "master". This lets us format the refspecs with a single xstrfmt. It's shorter, and easier to see what the final string looks like. We also update the misleading comment in this area (the local branch is named after the remote name, not after the branch name on the remote side). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Sep 24, 2015 at 17:07 UTC f28e3ab231d36c454445fe778ffb931920606109
1 file changed +20 -34
remote.c
+20 -34
@@ -293,56 +293,42 @@ static void read_remotes_file(struct remote *remote)
293 static void read_branches_file(struct remote *remote)
294 {
295 char *frag;
296 - struct strbuf branch = STRBUF_INIT;
297 - int n = 1000;
298 - FILE *f = fopen(git_path("branches/%.*s", n, remote->name), "r");
299 - char *s, *p;
300 - int len;
296 + struct strbuf buf = STRBUF_INIT;
297 + FILE *f = fopen(git_path("branches/%s", remote->name), "r");
298
299 if (!f)
300 return;
304 - s = fgets(buffer, BUF_SIZE, f);
305 - fclose(f);
306 - if (!s)
307 - return;
308 - while (isspace(*s))
309 - s++;
310 - if (!*s)
301 +
302 + strbuf_getline(&buf, f, '\n');
303 + strbuf_trim(&buf);
304 + if (!buf.len) {
305 + strbuf_release(&buf);
306 return;
307 + }
308 +
309 remote->origin = REMOTE_BRANCHES;
313 - p = s + strlen(s);
314 - while (isspace(p[-1]))
315 - *--p = 0;
316 - len = p - s;
317 - p = xmalloc(len + 1);
318 - strcpy(p, s);
310
311 /*
312 * The branches file would have URL and optionally
313 * #branch specified. The "master" (or specified) branch is
323 - * fetched and stored in the local branch of the same name.
314 + * fetched and stored in the local branch matching the
315 + * remote name.
316 */
325 - frag = strchr(p, '#');
326 - if (frag) {
317 + frag = strchr(buf.buf, '#');
318 + if (frag)
319 *(frag++) = '\0';
328 - strbuf_addf(&branch, "refs/heads/%s", frag);
329 - } else
330 - strbuf_addstr(&branch, "refs/heads/master");
320 + else
321 + frag = "master";
322 +
323 + add_url_alias(remote, strbuf_detach(&buf, NULL));
324 + add_fetch_refspec(remote, xstrfmt("refs/heads/%s:refs/heads/%s",
325 + frag, remote->name));
326
332 - strbuf_addf(&branch, ":refs/heads/%s", remote->name);
333 - add_url_alias(remote, p);
334 - add_fetch_refspec(remote, strbuf_detach(&branch, NULL));
327 /*
328 * Cogito compatible push: push current HEAD to remote #branch
329 * (master if missing)
330 */
339 - strbuf_init(&branch, 0);
340 - strbuf_addstr(&branch, "HEAD");
341 - if (frag)
342 - strbuf_addf(&branch, ":refs/heads/%s", frag);
343 - else
344 - strbuf_addstr(&branch, ":refs/heads/master");
345 - add_push_refspec(remote, strbuf_detach(&branch, NULL));
331 + add_push_refspec(remote, xstrfmt("HEAD:refs/heads/%s", frag));
332 remote->fetch_tags = 1; /* always auto-follow */
333 }
334