Merge branch 'rs/notes-merge-no-toctou'
authorJunio C Hamano <gitster@pobox.com>
Thu, 28 Jul 2016 17:34:41 +0000 (10:34 -0700)
committerJunio C Hamano <gitster@pobox.com>
Thu, 28 Jul 2016 17:34:41 +0000 (10:34 -0700)
"git notes merge" had a code to see if a path exists (and fails if
it does) and then open the path for writing (when it doesn't).
Replace it with open with O_EXCL.

* rs/notes-merge-no-toctou:
notes-merge: use O_EXCL to avoid overwriting existing files

1  2 
notes-merge.c
diff --combined notes-merge.c
index 1b58a14ebc30d47d942b868af1afdc8cc3195148,f00059520e62b1c726640437520db64e28abbac7..97fc42f64bcd7a46d4f7818b4ad96ee6622fbec1
@@@ -41,14 -41,14 +41,14 @@@ static int verify_notes_filepair(struc
        switch (p->status) {
        case DIFF_STATUS_MODIFIED:
                assert(p->one->mode == p->two->mode);
 -              assert(!is_null_sha1(p->one->sha1));
 -              assert(!is_null_sha1(p->two->sha1));
 +              assert(!is_null_oid(&p->one->oid));
 +              assert(!is_null_oid(&p->two->oid));
                break;
        case DIFF_STATUS_ADDED:
 -              assert(is_null_sha1(p->one->sha1));
 +              assert(is_null_oid(&p->one->oid));
                break;
        case DIFF_STATUS_DELETED:
 -              assert(is_null_sha1(p->two->sha1));
 +              assert(is_null_oid(&p->two->oid));
                break;
        default:
                return -1;
@@@ -142,27 -142,27 +142,27 @@@ static struct notes_merge_pair *diff_tr
                if (verify_notes_filepair(p, obj)) {
                        trace_printf("\t\tCannot merge entry '%s' (%c): "
                               "%.7s -> %.7s. Skipping!\n", p->one->path,
 -                             p->status, sha1_to_hex(p->one->sha1),
 -                             sha1_to_hex(p->two->sha1));
 +                             p->status, oid_to_hex(&p->one->oid),
 +                             oid_to_hex(&p->two->oid));
                        continue;
                }
                mp = find_notes_merge_pair_pos(changes, len, obj, 1, &occupied);
                if (occupied) {
                        /* We've found an addition/deletion pair */
                        assert(!hashcmp(mp->obj, obj));
 -                      if (is_null_sha1(p->one->sha1)) { /* addition */
 +                      if (is_null_oid(&p->one->oid)) { /* addition */
                                assert(is_null_sha1(mp->remote));
 -                              hashcpy(mp->remote, p->two->sha1);
 -                      } else if (is_null_sha1(p->two->sha1)) { /* deletion */
 +                              hashcpy(mp->remote, p->two->oid.hash);
 +                      } else if (is_null_oid(&p->two->oid)) { /* deletion */
                                assert(is_null_sha1(mp->base));
 -                              hashcpy(mp->base, p->one->sha1);
 +                              hashcpy(mp->base, p->one->oid.hash);
                        } else
                                assert(!"Invalid existing change recorded");
                } else {
                        hashcpy(mp->obj, obj);
 -                      hashcpy(mp->base, p->one->sha1);
 +                      hashcpy(mp->base, p->one->oid.hash);
                        hashcpy(mp->local, uninitialized);
 -                      hashcpy(mp->remote, p->two->sha1);
 +                      hashcpy(mp->remote, p->two->oid.hash);
                        len++;
                }
                trace_printf("\t\tStored remote change for %s: %.7s -> %.7s\n",
                       sha1_to_hex(mp->remote));
        }
        diff_flush(&opt);
 -      free_pathspec(&opt.pathspec);
 +      clear_pathspec(&opt.pathspec);
  
        *num_changes = len;
        return changes;
@@@ -203,21 -203,21 +203,21 @@@ static void diff_tree_local(struct note
                if (verify_notes_filepair(p, obj)) {
                        trace_printf("\t\tCannot merge entry '%s' (%c): "
                               "%.7s -> %.7s. Skipping!\n", p->one->path,
 -                             p->status, sha1_to_hex(p->one->sha1),
 -                             sha1_to_hex(p->two->sha1));
 +                             p->status, oid_to_hex(&p->one->oid),
 +                             oid_to_hex(&p->two->oid));
                        continue;
                }
                mp = find_notes_merge_pair_pos(changes, len, obj, 0, &match);
                if (!match) {
                        trace_printf("\t\tIgnoring local-only change for %s: "
                               "%.7s -> %.7s\n", sha1_to_hex(obj),
 -                             sha1_to_hex(p->one->sha1),
 -                             sha1_to_hex(p->two->sha1));
 +                             oid_to_hex(&p->one->oid),
 +                             oid_to_hex(&p->two->oid));
                        continue;
                }
  
                assert(!hashcmp(mp->obj, obj));
 -              if (is_null_sha1(p->two->sha1)) { /* deletion */
 +              if (is_null_oid(&p->two->oid)) { /* deletion */
                        /*
                         * Either this is a true deletion (1), or it is part
                         * of an A/D pair (2), or D/A pair (3):
                         */
                        if (!hashcmp(mp->local, uninitialized))
                                hashclr(mp->local);
 -              } else if (is_null_sha1(p->one->sha1)) { /* addition */
 +              } else if (is_null_oid(&p->one->oid)) { /* addition */
                        /*
                         * Either this is a true addition (1), or it is part
                         * of an A/D pair (2), or D/A pair (3):
                         */
                        assert(is_null_sha1(mp->local) ||
                               !hashcmp(mp->local, uninitialized));
 -                      hashcpy(mp->local, p->two->sha1);
 +                      hashcpy(mp->local, p->two->oid.hash);
                } else { /* modification */
                        /*
                         * This is a true modification. p->one->sha1 shall
                         * match mp->base, and mp->local shall be uninitialized.
                         * Set mp->local to p->two->sha1.
                         */
 -                      assert(!hashcmp(p->one->sha1, mp->base));
 +                      assert(!hashcmp(p->one->oid.hash, mp->base));
                        assert(!hashcmp(mp->local, uninitialized));
 -                      hashcpy(mp->local, p->two->sha1);
 +                      hashcpy(mp->local, p->two->oid.hash);
                }
                trace_printf("\t\tStored local change for %s: %.7s -> %.7s\n",
                       sha1_to_hex(mp->obj), sha1_to_hex(mp->base),
                       sha1_to_hex(mp->local));
        }
        diff_flush(&opt);
 -      free_pathspec(&opt.pathspec);
 +      clear_pathspec(&opt.pathspec);
  }
  
  static void check_notes_merge_worktree(struct notes_merge_options *o)
@@@ -298,12 -298,8 +298,8 @@@ static void write_buf_to_worktree(cons
        char *path = git_pathdup(NOTES_MERGE_WORKTREE "/%s", sha1_to_hex(obj));
        if (safe_create_leading_directories_const(path))
                die_errno("unable to create directory for '%s'", path);
-       if (file_exists(path))
-               die("found existing file at '%s'", path);
  
-       fd = open(path, O_WRONLY | O_TRUNC | O_CREAT, 0666);
-       if (fd < 0)
-               die_errno("failed to open '%s'", path);
+       fd = xopen(path, O_WRONLY | O_EXCL | O_CREAT, 0666);
  
        while (size > 0) {
                long ret = write_in_full(fd, buf, size);