From 5e8617f560968567c285bc2e9b0674f8f9d535cb Mon Sep 17 00:00:00 2001 From: Thomas Rast Date: Wed, 22 Feb 2012 20:34:22 +0100 Subject: bundle: put strbuf_readline_fd in strbuf.c with adjustments The comment even said that it should eventually go there. While at it, match the calling convention and name of the function to the strbuf_get*line family. So it now is strbuf_getwholeline_fd. Signed-off-by: Thomas Rast Signed-off-by: Junio C Hamano --- bundle.c | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-) (limited to 'bundle.c') diff --git a/bundle.c b/bundle.c index 8a1d53ba29..0542093ee8 100644 --- a/bundle.c +++ b/bundle.c @@ -23,23 +23,6 @@ static void add_to_ref_list(const unsigned char *sha1, const char *name, list->nr++; } -/* Eventually this should go to strbuf.[ch] */ -static int strbuf_readline_fd(struct strbuf *sb, int fd) -{ - strbuf_reset(sb); - - while (1) { - char ch; - ssize_t len = xread(fd, &ch, 1); - if (len <= 0) - return len; - strbuf_addch(sb, ch); - if (ch == '\n') - break; - } - return 0; -} - static int parse_bundle_header(int fd, struct bundle_header *header, const char *report_path) { @@ -47,7 +30,7 @@ static int parse_bundle_header(int fd, struct bundle_header *header, int status = 0; /* The bundle header begins with the signature */ - if (strbuf_readline_fd(&buf, fd) || + if (strbuf_getwholeline_fd(&buf, fd, '\n') || strcmp(buf.buf, bundle_signature)) { if (report_path) error("'%s' does not look like a v2 bundle file", @@ -57,7 +40,7 @@ static int parse_bundle_header(int fd, struct bundle_header *header, } /* The bundle header ends with an empty line */ - while (!strbuf_readline_fd(&buf, fd) && + while (!strbuf_getwholeline_fd(&buf, fd, '\n') && buf.len && buf.buf[0] != '\n') { unsigned char sha1[20]; int is_prereq = 0; -- cgit v1.2.1 From bc2fed496baa54ae99dede7da23dec938adbf0eb Mon Sep 17 00:00:00 2001 From: Thomas Rast Date: Wed, 22 Feb 2012 20:34:23 +0100 Subject: bundle: use a strbuf to scan the log for boundary commits The first part of the bundle header contains the boundary commits, and could be approximated by # v2 git bundle $(git rev-list --pretty=oneline --boundary | grep ^-) git-bundle actually spawns exactly this rev-list invocation, and does the grepping internally. There was a subtle bug in the latter step: it used fgets() with a 1024-byte buffer. If the user has sufficiently long subjects (e.g., by not adhering to the git oneline-subject convention in the first place), the 'oneline' format can easily overflow the buffer. fgets() then returns the rest of the line in the next call(s). If one of these remaining parts started with '-', git-bundle would mistakenly insert it into the bundle thinking it was a boundary commit. Fix it by using strbuf_getwholeline() instead, which handles arbitrary line lengths correctly. Note that on the receiving side in parse_bundle_header() we were already using strbuf_getwholeline_fd(), so that part is safe. Reported-by: Jannis Pohlmann Signed-off-by: Thomas Rast Signed-off-by: Junio C Hamano --- bundle.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) (limited to 'bundle.c') diff --git a/bundle.c b/bundle.c index 0542093ee8..4497343e56 100644 --- a/bundle.c +++ b/bundle.c @@ -234,7 +234,7 @@ int create_bundle(struct bundle_header *header, const char *path, const char **argv_boundary = xmalloc((argc + 4) * sizeof(const char *)); const char **argv_pack = xmalloc(6 * sizeof(const char *)); int i, ref_count = 0; - char buffer[1024]; + struct strbuf buf = STRBUF_INIT; struct rev_info revs; struct child_process rls; FILE *rls_fout; @@ -266,20 +266,21 @@ int create_bundle(struct bundle_header *header, const char *path, if (start_command(&rls)) return -1; rls_fout = xfdopen(rls.out, "r"); - while (fgets(buffer, sizeof(buffer), rls_fout)) { + while (strbuf_getwholeline(&buf, rls_fout, '\n') != EOF) { unsigned char sha1[20]; - if (buffer[0] == '-') { - write_or_die(bundle_fd, buffer, strlen(buffer)); - if (!get_sha1_hex(buffer + 1, sha1)) { + if (buf.len > 0 && buf.buf[0] == '-') { + write_or_die(bundle_fd, buf.buf, buf.len); + if (!get_sha1_hex(buf.buf + 1, sha1)) { struct object *object = parse_object(sha1); object->flags |= UNINTERESTING; - add_pending_object(&revs, object, buffer); + add_pending_object(&revs, object, buf.buf); } - } else if (!get_sha1_hex(buffer, sha1)) { + } else if (!get_sha1_hex(buf.buf, sha1)) { struct object *object = parse_object(sha1); object->flags |= SHOWN; } } + strbuf_release(&buf); fclose(rls_fout); if (finish_command(&rls)) return error("rev-list died"); -- cgit v1.2.1 From efe4be12490ab684786c48135731c4d2648bbecc Mon Sep 17 00:00:00 2001 From: Thomas Rast Date: Thu, 1 Mar 2012 22:40:51 +0100 Subject: bundle: keep around names passed to add_pending_object() The 'name' field passed to add_pending_object() is used to later deduplicate in object_array_remove_duplicates(). git-bundle had a bug in this area since 18449ab (git-bundle: avoid packing objects which are in the prerequisites, 2007-03-08): it passed the name of each boundary object in a static buffer. In other words, all that object_array_remove_duplicates() saw was the name of the *last* added boundary object. The recent switch to a strbuf in bc2fed4 (bundle: use a strbuf to scan the log for boundary commits, 2012-02-22) made this slightly worse: we now free the buffer at the end, so it is not even guaranteed that it still points into addressable memory by the time object_array_remove_ duplicates looks at it. On the plus side however, it was now detectable by valgrind. The fix is easy: pass a copy of the string to add_pending_object. Signed-off-by: Thomas Rast Signed-off-by: Junio C Hamano --- bundle.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'bundle.c') diff --git a/bundle.c b/bundle.c index 4497343e56..6c4695eb91 100644 --- a/bundle.c +++ b/bundle.c @@ -273,7 +273,7 @@ int create_bundle(struct bundle_header *header, const char *path, if (!get_sha1_hex(buf.buf + 1, sha1)) { struct object *object = parse_object(sha1); object->flags |= UNINTERESTING; - add_pending_object(&revs, object, buf.buf); + add_pending_object(&revs, object, xstrdup(buf.buf)); } } else if (!get_sha1_hex(buf.buf, sha1)) { struct object *object = parse_object(sha1); -- cgit v1.2.1 From 8a1e7eac689c53f15d986e8525484ca9bd640553 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Mon, 23 Apr 2012 19:30:30 +0700 Subject: i18n: bundle: mark strings for translation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- bundle.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) (limited to 'bundle.c') diff --git a/bundle.c b/bundle.c index d9cfd90534..6d21c77421 100644 --- a/bundle.c +++ b/bundle.c @@ -33,7 +33,7 @@ static int parse_bundle_header(int fd, struct bundle_header *header, if (strbuf_getwholeline_fd(&buf, fd, '\n') || strcmp(buf.buf, bundle_signature)) { if (report_path) - error("'%s' does not look like a v2 bundle file", + error(_("'%s' does not look like a v2 bundle file"), report_path); status = -1; goto abort; @@ -60,7 +60,7 @@ static int parse_bundle_header(int fd, struct bundle_header *header, (40 <= buf.len && !isspace(buf.buf[40])) || (!is_prereq && buf.len <= 40)) { if (report_path) - error("unrecognized header: %s%s (%d)", + error(_("unrecognized header: %s%s (%d)"), (is_prereq ? "-" : ""), buf.buf, (int)buf.len); status = -1; break; @@ -86,7 +86,7 @@ int read_bundle_header(const char *path, struct bundle_header *header) int fd = open(path, O_RDONLY); if (fd < 0) - return error("could not open '%s'", path); + return error(_("could not open '%s'"), path); return parse_bundle_header(fd, header, path); } @@ -137,7 +137,7 @@ int verify_bundle(struct bundle_header *header, int verbose) struct object_array refs; struct commit *commit; int i, ret = 0, req_nr; - const char *message = "Repository lacks these prerequisite commits:"; + const char *message = _("Repository lacks these prerequisite commits:"); init_revisions(&revs, NULL); for (i = 0; i < p->nr; i++) { @@ -161,7 +161,7 @@ int verify_bundle(struct bundle_header *header, int verbose) revs.leak_pending = 1; if (prepare_revision_walk(&revs)) - die("revision walk setup failed"); + die(_("revision walk setup failed")); i = req_nr; while (i && (commit = get_revision(&revs))) @@ -183,12 +183,16 @@ int verify_bundle(struct bundle_header *header, int verbose) struct ref_list *r; r = &header->references; - printf("The bundle contains %d ref%s\n", - r->nr, (1 < r->nr) ? "s" : ""); + printf_ln(Q_("The bundle contains %d ref", + "The bundle contains %d refs", + r->nr), + r->nr); list_refs(r, 0, NULL); r = &header->prerequisites; - printf("The bundle requires these %d ref%s\n", - r->nr, (1 < r->nr) ? "s" : ""); + printf_ln(Q_("The bundle requires this ref", + "The bundle requires these %d refs", + r->nr), + r->nr); list_refs(r, 0, NULL); } return ret; @@ -283,13 +287,13 @@ int create_bundle(struct bundle_header *header, const char *path, strbuf_release(&buf); fclose(rls_fout); if (finish_command(&rls)) - return error("rev-list died"); + return error(_("rev-list died")); /* write references */ argc = setup_revisions(argc, argv, &revs, NULL); if (argc > 1) - return error("unrecognized argument: %s'", argv[1]); + return error(_("unrecognized argument: %s'"), argv[1]); object_array_remove_duplicates(&revs.pending); @@ -324,7 +328,7 @@ int create_bundle(struct bundle_header *header, const char *path, * constraints. */ if (!(e->item->flags & SHOWN) && e->item->type == OBJ_COMMIT) { - warning("ref '%s' is excluded by the rev-list options", + warning(_("ref '%s' is excluded by the rev-list options"), e->name); free(ref); continue; @@ -369,7 +373,7 @@ int create_bundle(struct bundle_header *header, const char *path, free(ref); } if (!ref_count) - die ("Refusing to create empty bundle."); + die(_("Refusing to create empty bundle.")); /* end header */ write_or_die(bundle_fd, "\n", 1); @@ -387,7 +391,7 @@ int create_bundle(struct bundle_header *header, const char *path, rls.out = bundle_fd; rls.git_cmd = 1; if (start_command(&rls)) - return error("Could not spawn pack-objects"); + return error(_("Could not spawn pack-objects")); /* * start_command closed bundle_fd if it was > 1 @@ -405,10 +409,10 @@ int create_bundle(struct bundle_header *header, const char *path, } close(rls.in); if (finish_command(&rls)) - return error ("pack-objects died"); + return error(_("pack-objects died")); if (!bundle_to_stdout) { if (commit_lock_file(&lock)) - die_errno("cannot create '%s'", path); + die_errno(_("cannot create '%s'"), path); } return 0; } @@ -430,6 +434,6 @@ int unbundle(struct bundle_header *header, int bundle_fd, int flags) ip.no_stdout = 1; ip.git_cmd = 1; if (run_command(&ip)) - return error("index-pack died"); + return error(_("index-pack died")); return 0; } -- cgit v1.2.1 From 97afde15f58728138f8e178b2acb7c72473f7d96 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Thu, 26 Apr 2012 00:53:59 -0500 Subject: bundle: remove stray single-quote from error message After running rev-list --boundary to retrieve the list of boundary commits, "git bundle create" runs its own revision walk. If in this stage git encounters an unfamiliar option, it writes a message with an unbalanced quotation mark: error: unrecognized argument: --foo' Drop the stray quote to match the "unrecognized argument: %s" message used elsewhere and save translators some work. This is mostly a futureproofing measure: for now, the "rev-list --boundary" command catches most strange arguments on its own and the above message is not seen unless you try something esoteric like "git bundle create test.bundle --header HEAD". Reported-by: Junio C Hamano Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- bundle.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'bundle.c') diff --git a/bundle.c b/bundle.c index 6bf849740c..5364cbfa0f 100644 --- a/bundle.c +++ b/bundle.c @@ -256,7 +256,7 @@ int create_bundle(struct bundle_header *header, const char *path, argc = setup_revisions(argc, argv, &revs, NULL); if (argc > 1) - return error("unrecognized argument: %s'", argv[1]); + return error("unrecognized argument: %s", argv[1]); object_array_remove_duplicates(&revs.pending); -- cgit v1.2.1 From 8c3710fd3011d75b9768749bfaf5ac2f31b96085 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 4 Jun 2012 11:51:13 -0700 Subject: tweak "bundle verify" of a complete history A bundle that records a complete history without prerequiste is a useful way to sneakernet the sources of your configuration files under your home directory, etc. E.g. $ GIT_DIR=/srv/git/homesrc.git git bundle create x.bndl HEAD master Running "git bundle verify" on such a "complete" bundle, however, gives somewhat a funny output. $ git bundle verify x.bndl The bundle contains 2 refs b2611f37ebc7ed6435a72d77fbc5f8b48a7d7146 HEAD b2611f37ebc7ed6435a72d77fbc5f8b48a7d7146 refs/heads/master The bundle requires these 0 refs x.bndl is okay Reword "requires these 0 refs" to say "The bundle records a complete history" instead. Signed-off-by: Junio C Hamano --- bundle.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) (limited to 'bundle.c') diff --git a/bundle.c b/bundle.c index 8d31b98f58..8d12816b9d 100644 --- a/bundle.c +++ b/bundle.c @@ -188,12 +188,16 @@ int verify_bundle(struct bundle_header *header, int verbose) r->nr), r->nr); list_refs(r, 0, NULL); - r = &header->prerequisites; - printf_ln(Q_("The bundle requires this ref", - "The bundle requires these %d refs", - r->nr), - r->nr); - list_refs(r, 0, NULL); + if (!r->nr) { + printf_ln(_("The bundle records a complete history.")); + } else { + r = &header->prerequisites; + printf_ln(Q_("The bundle requires this ref", + "The bundle requires these %d refs", + r->nr), + r->nr); + list_refs(r, 0, NULL); + } } return ret; } -- cgit v1.2.1 From 71ba6b10f81ac8afcb0ba376f6691afe1269467c Mon Sep 17 00:00:00 2001 From: Lukas Fleischer Date: Thu, 7 Mar 2013 01:56:35 +0100 Subject: bundle: Fix "verify" output if history is complete A more informative message for "complete" bundles was added in commit 8c3710fd3011 (tweak "bundle verify" of a complete history, 2012-06-04). However, the prerequisites ref list is currently read *after* we check if it equals zero, which means we never actually use the number of prerequisite refs to decide when to print the newly introduced message. The code incorrectly uses the number of references recorded in the bundle instead. Signed-off-by: Lukas Fleischer Signed-off-by: Junio C Hamano --- bundle.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'bundle.c') diff --git a/bundle.c b/bundle.c index 8d12816b9d..65db53b6cb 100644 --- a/bundle.c +++ b/bundle.c @@ -188,10 +188,10 @@ int verify_bundle(struct bundle_header *header, int verbose) r->nr), r->nr); list_refs(r, 0, NULL); + r = &header->prerequisites; if (!r->nr) { printf_ln(_("The bundle records a complete history.")); } else { - r = &header->prerequisites; printf_ln(Q_("The bundle requires this ref", "The bundle requires these %d refs", r->nr), -- cgit v1.2.1 From a02ffe0e1a19c9594af3e81f4e04b46eabe4755a Mon Sep 17 00:00:00 2001 From: Lukas Fleischer Date: Fri, 8 Mar 2013 19:01:26 +0100 Subject: bundle: Add colons to list headings in "verify" These slightly improve the reading flow by making it obvious that a list follows. Also, make the wording of both headings consistent by changing "contains %d ref(s)" to "contains this ref"/"contains these %d refs". Signed-off-by: Lukas Fleischer Signed-off-by: Junio C Hamano --- bundle.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'bundle.c') diff --git a/bundle.c b/bundle.c index 65db53b6cb..6210a6be89 100644 --- a/bundle.c +++ b/bundle.c @@ -183,8 +183,8 @@ int verify_bundle(struct bundle_header *header, int verbose) struct ref_list *r; r = &header->references; - printf_ln(Q_("The bundle contains %d ref", - "The bundle contains %d refs", + printf_ln(Q_("The bundle contains this ref:", + "The bundle contains these %d refs:", r->nr), r->nr); list_refs(r, 0, NULL); @@ -192,8 +192,8 @@ int verify_bundle(struct bundle_header *header, int verbose) if (!r->nr) { printf_ln(_("The bundle records a complete history.")); } else { - printf_ln(Q_("The bundle requires this ref", - "The bundle requires these %d refs", + printf_ln(Q_("The bundle requires this ref:", + "The bundle requires these %d refs:", r->nr), r->nr); list_refs(r, 0, NULL); -- cgit v1.2.1 From 75a95490474ab6e991cbbbd10d980498a9109648 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 17 Mar 2013 04:22:36 -0400 Subject: avoid segfaults on parse_object failure Many call-sites of parse_object assume that they will get a non-NULL return value; this is not the case if we encounter an error while parsing the object. This patch adds a wrapper function around parse_object that handles dying automatically, and uses it anywhere we immediately try to access the return value as a non-NULL pointer (i.e., anywhere that we would currently segfault). This wrapper may also be useful in other places. The most obvious one is code like: o = parse_object(sha1); if (!o) die(...); However, these should not be mechanically converted to parse_object_or_die, as the die message is sometimes customized. Later patches can address these sites on a case-by-case basis. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- bundle.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'bundle.c') diff --git a/bundle.c b/bundle.c index 8d12816b9d..26ceebdb4b 100644 --- a/bundle.c +++ b/bundle.c @@ -279,12 +279,12 @@ int create_bundle(struct bundle_header *header, const char *path, if (buf.len > 0 && buf.buf[0] == '-') { write_or_die(bundle_fd, buf.buf, buf.len); if (!get_sha1_hex(buf.buf + 1, sha1)) { - struct object *object = parse_object(sha1); + struct object *object = parse_object_or_die(sha1, buf.buf); object->flags |= UNINTERESTING; add_pending_object(&revs, object, xstrdup(buf.buf)); } } else if (!get_sha1_hex(buf.buf, sha1)) { - struct object *object = parse_object(sha1); + struct object *object = parse_object_or_die(sha1, buf.buf); object->flags |= SHOWN; } } @@ -361,7 +361,7 @@ int create_bundle(struct bundle_header *header, const char *path, * end up triggering "empty bundle" * error. */ - obj = parse_object(sha1); + obj = parse_object_or_die(sha1, e->name); obj->flags |= SHOWN; add_pending_object(&revs, obj, e->name); } -- cgit v1.2.1 From 5446e33f35d6b27b6b760a59a6ded33fbd555190 Mon Sep 17 00:00:00 2001 From: Lukas Fleischer Date: Sun, 7 Apr 2013 13:53:15 +0200 Subject: bundle: Accept prerequisites without commit messages While explicitly stating that the commit message in a prerequisite line is optional, we required all lines with 40 or more characters to contain a space after the object name, bailing out if a line consisted of an object name only. This was to allow bundling a history to a commit without an message, but the code forgot that it already called rtrim() to remove that whitespace. As a workaround, only check for SP when the line has more than 40 characters. Signed-off-by: Lukas Fleischer Signed-off-by: Junio C Hamano --- bundle.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'bundle.c') diff --git a/bundle.c b/bundle.c index 505e07e934..4b0e5cd51b 100644 --- a/bundle.c +++ b/bundle.c @@ -57,7 +57,7 @@ static int parse_bundle_header(int fd, struct bundle_header *header, * followed by SP and subject line. */ if (get_sha1_hex(buf.buf, sha1) || - (40 <= buf.len && !isspace(buf.buf[40])) || + (buf.len > 40 && !isspace(buf.buf[40])) || (!is_prereq && buf.len <= 40)) { if (report_path) error(_("unrecognized header: %s%s (%d)"), -- cgit v1.2.1 From 31faeb2088ef35d0108ad81df3550513d6cec798 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sat, 25 May 2013 11:08:14 +0200 Subject: object_array_entry: fix memory handling of the name field Previously, the memory management of the object_array_entry::name field was inconsistent and undocumented. object_array_entries are ultimately created by a single function, add_object_array_with_mode(), which has an argument "const char *name". This function used to simply set the name field to reference the string pointed to by the name parameter, and nobody on the object_array side ever freed the memory. Thus, it assumed that the memory for the name field would be managed by the caller, and that the lifetime of that string would be at least as long as the lifetime of the object_array_entry. But callers were inconsistent: * Some passed pointers to constant strings or argv entries, which was OK. * Some passed pointers to newly-allocated memory, but didn't arrange for the memory ever to be freed. * Some passed the return value of sha1_to_hex(), which is a pointer to a statically-allocated buffer that can be overwritten at any time. * Some passed pointers to refnames that they received from a for_each_ref()-type iteration, but the lifetimes of such refnames is not guaranteed by the refs API. Bring consistency to this mess by changing object_array to make its own copy for the object_array_entry::name field and free this memory when an object_array_entry is deleted from the array. Many callers were passing the empty string as the name parameter, so as a performance optimization, treat the empty string specially. Instead of making a copy, store a pointer to a statically-allocated empty string to object_array_entry::name. When deleting such an entry, skip the free(). Change the callers that were already passing copies to add_object_array_with_mode() to either skip the copy, or (if the memory needed to be allocated anyway) freeing the memory itself. A part of this commit effectively reverts 70d26c6e76 read_revisions_from_stdin: make copies for handle_revision_arg because the copying introduced by that commit (which is still necessary) is now done at a deeper level. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- bundle.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'bundle.c') diff --git a/bundle.c b/bundle.c index 4b0e5cd51b..3d64311373 100644 --- a/bundle.c +++ b/bundle.c @@ -281,7 +281,7 @@ int create_bundle(struct bundle_header *header, const char *path, if (!get_sha1_hex(buf.buf + 1, sha1)) { struct object *object = parse_object_or_die(sha1, buf.buf); object->flags |= UNINTERESTING; - add_pending_object(&revs, object, xstrdup(buf.buf)); + add_pending_object(&revs, object, buf.buf); } } else if (!get_sha1_hex(buf.buf, sha1)) { struct object *object = parse_object_or_die(sha1, buf.buf); -- cgit v1.2.1