transport: use strbufs for status table "quickref" strings

We generate range strings like "1234abcd...5678efab" for use in the the fetch and push status tables. We use fixed-size buffers along with strcat to do so. These aren't buggy, as our manual size computation is correct, but there's nothing checking that this is so. Let's switch them to strbufs instead, which are obviously correct, and make it easier to audit the code base for problematic calls to strcat(). 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 bd22d4ffbc10052fef1a6c52aec066ee64236340
2 files changed +19 -16
builtin/fetch.c
+12 -10
@@ -528,36 +528,38 @@ static int update_local_ref(struct ref *ref,
528 }
529
530 if (in_merge_bases(current, updated)) {
531 - char quickref[83];
531 + struct strbuf quickref = STRBUF_INIT;
532 int r;
533 - strcpy(quickref, find_unique_abbrev(current->object.sha1, DEFAULT_ABBREV));
534 - strcat(quickref, "..");
535 - strcat(quickref, find_unique_abbrev(ref->new_sha1, DEFAULT_ABBREV));
533 + strbuf_add_unique_abbrev(&quickref, current->object.sha1, DEFAULT_ABBREV);
534 + strbuf_addstr(&quickref, "..");
535 + strbuf_add_unique_abbrev(&quickref, ref->new_sha1, DEFAULT_ABBREV);
536 if ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&
537 (recurse_submodules != RECURSE_SUBMODULES_ON))
538 check_for_new_submodule_commits(ref->new_sha1);
539 r = s_update_ref("fast-forward", ref, 1);
540 strbuf_addf(display, "%c %-*s %-*s -> %s%s",
541 r ? '!' : ' ',
542 - TRANSPORT_SUMMARY_WIDTH, quickref,
542 + TRANSPORT_SUMMARY_WIDTH, quickref.buf,
543 REFCOL_WIDTH, remote, pretty_ref,
544 r ? _(" (unable to update local ref)") : "");
545 + strbuf_release(&quickref);
546 return r;
547 } else if (force || ref->force) {
547 - char quickref[84];
548 + struct strbuf quickref = STRBUF_INIT;
549 int r;
549 - strcpy(quickref, find_unique_abbrev(current->object.sha1, DEFAULT_ABBREV));
550 - strcat(quickref, "...");
551 - strcat(quickref, find_unique_abbrev(ref->new_sha1, DEFAULT_ABBREV));
550 + strbuf_add_unique_abbrev(&quickref, current->object.sha1, DEFAULT_ABBREV);
551 + strbuf_addstr(&quickref, "...");
552 + strbuf_add_unique_abbrev(&quickref, ref->new_sha1, DEFAULT_ABBREV);
553 if ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&
554 (recurse_submodules != RECURSE_SUBMODULES_ON))
555 check_for_new_submodule_commits(ref->new_sha1);
556 r = s_update_ref("forced-update", ref, 1);
557 strbuf_addf(display, "%c %-*s %-*s -> %s (%s)",
558 r ? '!' : '+',
558 - TRANSPORT_SUMMARY_WIDTH, quickref,
559 + TRANSPORT_SUMMARY_WIDTH, quickref.buf,
560 REFCOL_WIDTH, remote, pretty_ref,
561 r ? _("unable to update local ref") : _("forced update"));
562 + strbuf_release(&quickref);
563 return r;
564 } else {
565 strbuf_addf(display, "! %-*s %-*s -> %s %s",
transport.c
+7 -6
@@ -654,23 +654,24 @@ static void print_ok_ref_status(struct ref *ref, int porcelain)
654 "[new branch]"),
655 ref, ref->peer_ref, NULL, porcelain);
656 else {
657 - char quickref[84];
657 + struct strbuf quickref = STRBUF_INIT;
658 char type;
659 const char *msg;
660
661 - strcpy(quickref, status_abbrev(ref->old_sha1));
661 + strbuf_addstr(&quickref, status_abbrev(ref->old_sha1));
662 if (ref->forced_update) {
663 - strcat(quickref, "...");
663 + strbuf_addstr(&quickref, "...");
664 type = '+';
665 msg = "forced update";
666 } else {
667 - strcat(quickref, "..");
667 + strbuf_addstr(&quickref, "..");
668 type = ' ';
669 msg = NULL;
670 }
671 - strcat(quickref, status_abbrev(ref->new_sha1));
671 + strbuf_addstr(&quickref, status_abbrev(ref->new_sha1));
672
673 - print_ref_status(type, quickref, ref, ref->peer_ref, msg, porcelain);
673 + print_ref_status(type, quickref.buf, ref, ref->peer_ref, msg, porcelain);
674 + strbuf_release(&quickref);
675 }
676 }
677