use sha1_to_hex_r() instead of strcpy

Before sha1_to_hex_r() existed, a simple way to get hex sha1 into a buffer was with: strcpy(buf, sha1_to_hex(sha1)); This isn't wrong (assuming the buf is 41 characters), but it makes auditing the code base for bad strcpy() calls harder, as these become false positives. Let's convert them to sha1_to_hex_r(), and likewise for some calls to find_unique_abbrev(). While we're here, we'll double-check that all of the buffers are correctly sized, and use the more obvious GIT_SHA1_HEXSZ constant. 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:08 UTC d59f765ac9b3d6fc2e6bea262222b80493055f12
6 files changed +31 -29
builtin/blame.c
+4 -4
@@ -1867,9 +1867,9 @@ static void emit_porcelain(struct scoreboard *sb, struct blame_entry *ent,
1867 int cnt;
1868 const char *cp;
1869 struct origin *suspect = ent->suspect;
1870 - char hex[41];
1870 + char hex[GIT_SHA1_HEXSZ + 1];
1871
1872 - strcpy(hex, sha1_to_hex(suspect->commit->object.sha1));
1872 + sha1_to_hex_r(hex, suspect->commit->object.sha1);
1873 printf("%s %d %d %d\n",
1874 hex,
1875 ent->s_lno + 1,
@@ -1905,11 +1905,11 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
1905 const char *cp;
1906 struct origin *suspect = ent->suspect;
1907 struct commit_info ci;
1908 - char hex[41];
1908 + char hex[GIT_SHA1_HEXSZ + 1];
1909 int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
1910
1911 get_commit_info(suspect->commit, &ci, 1);
1912 - strcpy(hex, sha1_to_hex(suspect->commit->object.sha1));
1912 + sha1_to_hex_r(hex, suspect->commit->object.sha1);
1913
1914 cp = nth_line(sb, ent->lno);
1915 for (cnt = 0; cnt < ent->num_lines; cnt++) {
builtin/merge-index.c
+2 -2
@@ -9,7 +9,7 @@ static int merge_entry(int pos, const char *path)
9 {
10 int found;
11 const char *arguments[] = { pgm, "", "", "", path, "", "", "", NULL };
12 - char hexbuf[4][60];
12 + char hexbuf[4][GIT_SHA1_HEXSZ + 1];
13 char ownbuf[4][60];
14
15 if (pos >= active_nr)
@@ -22,7 +22,7 @@ static int merge_entry(int pos, const char *path)
22 if (strcmp(ce->name, path))
23 break;
24 found++;
25 - strcpy(hexbuf[stage], sha1_to_hex(ce->sha1));
25 + sha1_to_hex_r(hexbuf[stage], ce->sha1);
26 xsnprintf(ownbuf[stage], sizeof(ownbuf[stage]), "%o", ce->ce_mode);
27 arguments[stage] = hexbuf[stage];
28 arguments[stage + 4] = ownbuf[stage];
builtin/merge.c
+10 -10
@@ -1319,13 +1319,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
1319 if (verify_signatures) {
1320 for (p = remoteheads; p; p = p->next) {
1321 struct commit *commit = p->item;
1322 - char hex[41];
1322 + char hex[GIT_SHA1_HEXSZ + 1];
1323 struct signature_check signature_check;
1324 memset(&signature_check, 0, sizeof(signature_check));
1325
1326 check_commit_signature(commit, &signature_check);
1327
1328 - strcpy(hex, find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV));
1328 + find_unique_abbrev_r(hex, commit->object.sha1, DEFAULT_ABBREV);
1329 switch (signature_check.result) {
1330 case 'G':
1331 break;
@@ -1415,15 +1415,15 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
1415 /* Again the most common case of merging one remote. */
1416 struct strbuf msg = STRBUF_INIT;
1417 struct commit *commit;
1418 - char hex[41];
1418
1420 - strcpy(hex, find_unique_abbrev(head_commit->object.sha1, DEFAULT_ABBREV));
1421 -
1422 - if (verbosity >= 0)
1423 - printf(_("Updating %s..%s\n"),
1424 - hex,
1425 - find_unique_abbrev(remoteheads->item->object.sha1,
1426 - DEFAULT_ABBREV));
1419 + if (verbosity >= 0) {
1420 + char from[GIT_SHA1_HEXSZ + 1], to[GIT_SHA1_HEXSZ + 1];
1421 + find_unique_abbrev_r(from, head_commit->object.sha1,
1422 + DEFAULT_ABBREV);
1423 + find_unique_abbrev_r(to, remoteheads->item->object.sha1,
1424 + DEFAULT_ABBREV);
1425 + printf(_("Updating %s..%s\n"), from, to);
1426 + }
1427 strbuf_addstr(&msg, "Fast-forward");
1428 if (have_message)
1429 strbuf_addstr(&msg,
builtin/receive-pack.c
+9 -6
@@ -1071,8 +1071,11 @@ static void check_aliased_update(struct command *cmd, struct string_list *list)
1071 const char *dst_name;
1072 struct string_list_item *item;
1073 struct command *dst_cmd;
1074 - unsigned char sha1[20];
1075 - char cmd_oldh[41], cmd_newh[41], dst_oldh[41], dst_newh[41];
1074 + unsigned char sha1[GIT_SHA1_RAWSZ];
1075 + char cmd_oldh[GIT_SHA1_HEXSZ + 1],
1076 + cmd_newh[GIT_SHA1_HEXSZ + 1],
1077 + dst_oldh[GIT_SHA1_HEXSZ + 1],
1078 + dst_newh[GIT_SHA1_HEXSZ + 1];
1079 int flag;
1080
1081 strbuf_addf(&buf, "%s%s", get_git_namespace(), cmd->ref_name);
@@ -1103,10 +1106,10 @@ static void check_aliased_update(struct command *cmd, struct string_list *list)
1106
1107 dst_cmd->skip_update = 1;
1108
1106 - strcpy(cmd_oldh, find_unique_abbrev(cmd->old_sha1, DEFAULT_ABBREV));
1107 - strcpy(cmd_newh, find_unique_abbrev(cmd->new_sha1, DEFAULT_ABBREV));
1108 - strcpy(dst_oldh, find_unique_abbrev(dst_cmd->old_sha1, DEFAULT_ABBREV));
1109 - strcpy(dst_newh, find_unique_abbrev(dst_cmd->new_sha1, DEFAULT_ABBREV));
1109 + find_unique_abbrev_r(cmd_oldh, cmd->old_sha1, DEFAULT_ABBREV);
1110 + find_unique_abbrev_r(cmd_newh, cmd->new_sha1, DEFAULT_ABBREV);
1111 + find_unique_abbrev_r(dst_oldh, dst_cmd->old_sha1, DEFAULT_ABBREV);
1112 + find_unique_abbrev_r(dst_newh, dst_cmd->new_sha1, DEFAULT_ABBREV);
1113 rp_error("refusing inconsistent update between symref '%s' (%s..%s) and"
1114 " its target '%s' (%s..%s)",
1115 cmd->ref_name, cmd_oldh, cmd_newh,
builtin/rev-list.c
+2 -2
@@ -217,7 +217,7 @@ static void print_var_int(const char *var, int val)
217 static int show_bisect_vars(struct rev_list_info *info, int reaches, int all)
218 {
219 int cnt, flags = info->flags;
220 - char hex[41] = "";
220 + char hex[GIT_SHA1_HEXSZ + 1] = "";
221 struct commit_list *tried;
222 struct rev_info *revs = info->revs;
223
@@ -242,7 +242,7 @@ static int show_bisect_vars(struct rev_list_info *info, int reaches, int all)
242 cnt = reaches;
243
244 if (revs->commits)
245 - strcpy(hex, sha1_to_hex(revs->commits->item->object.sha1));
245 + sha1_to_hex_r(hex, revs->commits->item->object.sha1);
246
247 if (flags & BISECT_SHOW_ALL) {
248 traverse_commit_list(revs, show_commit, show_object, info);
diff.c
+4 -5
@@ -322,7 +322,7 @@ static struct diff_tempfile {
322 */
323 const char *name;
324
325 - char hex[41];
325 + char hex[GIT_SHA1_HEXSZ + 1];
326 char mode[10];
327
328 /*
@@ -2878,8 +2878,7 @@ static void prep_temp_blob(const char *path, struct diff_tempfile *temp,
2878 die_errno("unable to write temp-file");
2879 close_tempfile(&temp->tempfile);
2880 temp->name = get_tempfile_path(&temp->tempfile);
2881 - strcpy(temp->hex, sha1_to_hex(sha1));
2882 - temp->hex[40] = 0;
2881 + sha1_to_hex_r(temp->hex, sha1);
2882 xsnprintf(temp->mode, sizeof(temp->mode), "%06o", mode);
2883 strbuf_release(&buf);
2884 strbuf_release(&template);
@@ -2926,9 +2925,9 @@ static struct diff_tempfile *prepare_temp_file(const char *name,
2925 /* we can borrow from the file in the work tree */
2926 temp->name = name;
2927 if (!one->sha1_valid)
2929 - strcpy(temp->hex, sha1_to_hex(null_sha1));
2928 + sha1_to_hex_r(temp->hex, null_sha1);
2929 else
2931 - strcpy(temp->hex, sha1_to_hex(one->sha1));
2930 + sha1_to_hex_r(temp->hex, one->sha1);
2931 /* Even though we may sometimes borrow the
2932 * contents from the work tree, we always want
2933 * one->mode. mode is trustworthy even when