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>

diff --git a/builtin/blame.c b/builtin/blame.c
index 4db01c195cd27719023eeb599e76428ad752c9c9..e253ac0dcb858c9911bf3dbf9dcb11dbcbfbfc84 100644 (file)
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1867,9 +1867,9 @@ static void emit_porcelain(struct scoreboard *sb, struct blame_entry *ent,
        int cnt;
        const char *cp;
        struct origin *suspect = ent->suspect;
-       char hex[41];
+       char hex[GIT_SHA1_HEXSZ + 1];
 
-       strcpy(hex, sha1_to_hex(suspect->commit->object.sha1));
+       sha1_to_hex_r(hex, suspect->commit->object.sha1);
        printf("%s %d %d %d\n",
               hex,
               ent->s_lno + 1,
@@ -1905,11 +1905,11 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
        const char *cp;
        struct origin *suspect = ent->suspect;
        struct commit_info ci;
-       char hex[41];
+       char hex[GIT_SHA1_HEXSZ + 1];
        int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
 
        get_commit_info(suspect->commit, &ci, 1);
-       strcpy(hex, sha1_to_hex(suspect->commit->object.sha1));
+       sha1_to_hex_r(hex, suspect->commit->object.sha1);
 
        cp = nth_line(sb, ent->lno);
        for (cnt = 0; cnt < ent->num_lines; cnt++) {

diff --git a/builtin/merge-index.c b/builtin/merge-index.c
index 1d6611191791ff66822b4a534e3c8dd1a6b9e473..1c3427c36caa40e8740b090c44a048201e81c706 100644 (file)
--- a/builtin/merge-index.c
+++ b/builtin/merge-index.c
@@ -9,7 +9,7 @@ static int merge_entry(int pos, const char *path)
 {
        int found;
        const char *arguments[] = { pgm, "", "", "", path, "", "", "", NULL };
-       char hexbuf[4][60];
+       char hexbuf[4][GIT_SHA1_HEXSZ + 1];
        char ownbuf[4][60];
 
        if (pos >= active_nr)
@@ -22,7 +22,7 @@ static int merge_entry(int pos, const char *path)
                if (strcmp(ce->name, path))
                        break;
                found++;
-               strcpy(hexbuf[stage], sha1_to_hex(ce->sha1));
+               sha1_to_hex_r(hexbuf[stage], ce->sha1);
                xsnprintf(ownbuf[stage], sizeof(ownbuf[stage]), "%o", ce->ce_mode);
                arguments[stage] = hexbuf[stage];
                arguments[stage + 4] = ownbuf[stage];

diff --git a/builtin/merge.c b/builtin/merge.c
index a0edacab20bbe55458bb114bd00d618b1bc41a7a..a0a93282922ebb8c58b79ff365d859500041351a 100644 (file)
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1319,13 +1319,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
        if (verify_signatures) {
                for (p = remoteheads; p; p = p->next) {
                        struct commit *commit = p->item;
-                       char hex[41];
+                       char hex[GIT_SHA1_HEXSZ + 1];
                        struct signature_check signature_check;
                        memset(&signature_check, 0, sizeof(signature_check));
 
                        check_commit_signature(commit, &signature_check);
 
-                       strcpy(hex, find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV));
+                       find_unique_abbrev_r(hex, commit->object.sha1, DEFAULT_ABBREV);
                        switch (signature_check.result) {
                        case 'G':
                                break;
@@ -1415,15 +1415,15 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
                /* Again the most common case of merging one remote. */
                struct strbuf msg = STRBUF_INIT;
                struct commit *commit;
-               char hex[41];
 
-               strcpy(hex, find_unique_abbrev(head_commit->object.sha1, DEFAULT_ABBREV));
-
-               if (verbosity >= 0)
-                       printf(_("Updating %s..%s\n"),
-                               hex,
-                               find_unique_abbrev(remoteheads->item->object.sha1,
-                               DEFAULT_ABBREV));
+               if (verbosity >= 0) {
+                       char from[GIT_SHA1_HEXSZ + 1], to[GIT_SHA1_HEXSZ + 1];
+                       find_unique_abbrev_r(from, head_commit->object.sha1,
+                                             DEFAULT_ABBREV);
+                       find_unique_abbrev_r(to, remoteheads->item->object.sha1,
+                                             DEFAULT_ABBREV);
+                       printf(_("Updating %s..%s\n"), from, to);
+               }
                strbuf_addstr(&msg, "Fast-forward");
                if (have_message)
                        strbuf_addstr(&msg,

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 04d2bdf3f34212239d88120ab1fd2f5c535f8773..39eb0646fd63900da8fc7796805078036864c3c5 100644 (file)
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1071,8 +1071,11 @@ static void check_aliased_update(struct command *cmd, struct string_list *list)
        const char *dst_name;
        struct string_list_item *item;
        struct command *dst_cmd;
-       unsigned char sha1[20];
-       char cmd_oldh[41], cmd_newh[41], dst_oldh[41], dst_newh[41];
+       unsigned char sha1[GIT_SHA1_RAWSZ];
+       char cmd_oldh[GIT_SHA1_HEXSZ + 1],
+            cmd_newh[GIT_SHA1_HEXSZ + 1],
+            dst_oldh[GIT_SHA1_HEXSZ + 1],
+            dst_newh[GIT_SHA1_HEXSZ + 1];
        int flag;
 
        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)
 
        dst_cmd->skip_update = 1;
 
-       strcpy(cmd_oldh, find_unique_abbrev(cmd->old_sha1, DEFAULT_ABBREV));
-       strcpy(cmd_newh, find_unique_abbrev(cmd->new_sha1, DEFAULT_ABBREV));
-       strcpy(dst_oldh, find_unique_abbrev(dst_cmd->old_sha1, DEFAULT_ABBREV));
-       strcpy(dst_newh, find_unique_abbrev(dst_cmd->new_sha1, DEFAULT_ABBREV));
+       find_unique_abbrev_r(cmd_oldh, cmd->old_sha1, DEFAULT_ABBREV);
+       find_unique_abbrev_r(cmd_newh, cmd->new_sha1, DEFAULT_ABBREV);
+       find_unique_abbrev_r(dst_oldh, dst_cmd->old_sha1, DEFAULT_ABBREV);
+       find_unique_abbrev_r(dst_newh, dst_cmd->new_sha1, DEFAULT_ABBREV);
        rp_error("refusing inconsistent update between symref '%s' (%s..%s) and"
                 " its target '%s' (%s..%s)",
                 cmd->ref_name, cmd_oldh, cmd_newh,

diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index d80d1ed35944144aa0aeeeb1b0e08c9efdec6523..491d298fa25cebb1afd9b33cbbd099efe72629bc 100644 (file)
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -217,7 +217,7 @@ static void print_var_int(const char *var, int val)
 static int show_bisect_vars(struct rev_list_info *info, int reaches, int all)
 {
        int cnt, flags = info->flags;
-       char hex[41] = "";
+       char hex[GIT_SHA1_HEXSZ + 1] = "";
        struct commit_list *tried;
        struct rev_info *revs = info->revs;
 
@@ -242,7 +242,7 @@ static int show_bisect_vars(struct rev_list_info *info, int reaches, int all)
                cnt = reaches;
 
        if (revs->commits)
-               strcpy(hex, sha1_to_hex(revs->commits->item->object.sha1));
+               sha1_to_hex_r(hex, revs->commits->item->object.sha1);
 
        if (flags & BISECT_SHOW_ALL) {
                traverse_commit_list(revs, show_commit, show_object, info);

diff --git a/diff.c b/diff.c
index 788e371decee39fb2aa0048c8b93f322d7ffa832..2a37378ec82773c337505b7f0c4309bf8f4b72ca 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -322,7 +322,7 @@ static struct diff_tempfile {
         */
        const char *name;
 
-       char hex[41];
+       char hex[GIT_SHA1_HEXSZ + 1];
        char mode[10];
 
        /*
@@ -2878,8 +2878,7 @@ static void prep_temp_blob(const char *path, struct diff_tempfile *temp,
                die_errno("unable to write temp-file");
        close_tempfile(&temp->tempfile);
        temp->name = get_tempfile_path(&temp->tempfile);
-       strcpy(temp->hex, sha1_to_hex(sha1));
-       temp->hex[40] = 0;
+       sha1_to_hex_r(temp->hex, sha1);
        xsnprintf(temp->mode, sizeof(temp->mode), "%06o", mode);
        strbuf_release(&buf);
        strbuf_release(&template);
@@ -2926,9 +2925,9 @@ static struct diff_tempfile *prepare_temp_file(const char *name,
                        /* we can borrow from the file in the work tree */
                        temp->name = name;
                        if (!one->sha1_valid)
-                               strcpy(temp->hex, sha1_to_hex(null_sha1));
+                               sha1_to_hex_r(temp->hex, null_sha1);
                        else
-                               strcpy(temp->hex, sha1_to_hex(one->sha1));
+                               sha1_to_hex_r(temp->hex, one->sha1);
                        /* Even though we may sometimes borrow the
                         * contents from the work tree, we always want
                         * one->mode.  mode is trustworthy even when