shallow: use struct 'shallow_lock' for additional safety

In previous patches, the functions 'commit_shallow_file' and
'rollback_shallow_file' were introduced to reset the shallowness
validity checks on a repository after potentially modifying
'.git/shallow'.

These functions can be made safer by wrapping the 'struct lockfile *' in
a new type, 'shallow_lock', so that they cannot be called with a raw
lock (and potentially misused by other code that happens to possess a
lockfile, but has nothing to do with shallowness).

This patch introduces that type as a thin wrapper around 'struct
lockfile', and updates the two aforementioned functions and their
callers to use it.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 8e3f9bc35f49b97279b5165c4d8a3e16cb7f22f0..73cebc2f552136d1e6c2907f4fd0b293009f78d6 100644 (file)
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -856,7 +856,7 @@ static void refuse_unconfigured_deny_delete_current(void)
 static int command_singleton_iterator(void *cb_data, struct object_id *oid);
 static int update_shallow_ref(struct command *cmd, struct shallow_info *si)
 {
-       struct lock_file shallow_lock = LOCK_INIT;
+       struct shallow_lock shallow_lock = SHALLOW_LOCK_INIT;
        struct oid_array extra = OID_ARRAY_INIT;
        struct check_connected_options opt = CHECK_CONNECTED_INIT;
        uint32_t mask = 1 << (cmd->index % 32);

diff --git a/fetch-pack.c b/fetch-pack.c
index 401d028e4172c6c4ace91c76b1e2edeccdfdfd0c..27d2d08cc033a8e302e8dd7589791e3ef0bec7fe 100644 (file)
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -35,7 +35,7 @@ static int fetch_fsck_objects = -1;
 static int transfer_fsck_objects = -1;
 static int agent_supported;
 static int server_supports_filtering;
-static struct lock_file shallow_lock;
+static struct shallow_lock shallow_lock;
 static const char *alternate_shallow_file;
 static struct strbuf fsck_msg_types = STRBUF_INIT;
 

diff --git a/shallow.c b/shallow.c
index 76e00893fec302d9e801d83ea06ece11be1d1ecb..7e0a41ea68bdc30b28d67c83425dd207283568d5 100644 (file)
--- a/shallow.c
+++ b/shallow.c
@@ -92,16 +92,16 @@ static void reset_repository_shallow(struct repository *r)
        stat_validity_clear(r->parsed_objects->shallow_stat);
 }
 
-int commit_shallow_file(struct repository *r, struct lock_file *lk)
+int commit_shallow_file(struct repository *r, struct shallow_lock *lk)
 {
-       int res = commit_lock_file(lk);
+       int res = commit_lock_file(&lk->lock);
        reset_repository_shallow(r);
        return res;
 }
 
-void rollback_shallow_file(struct repository *r, struct lock_file *lk)
+void rollback_shallow_file(struct repository *r, struct shallow_lock *lk)
 {
-       rollback_lock_file(lk);
+       rollback_lock_file(&lk->lock);
        reset_repository_shallow(r);
 }
 
@@ -366,22 +366,22 @@ const char *setup_temporary_shallow(const struct oid_array *extra)
        return "";
 }
 
-void setup_alternate_shallow(struct lock_file *shallow_lock,
+void setup_alternate_shallow(struct shallow_lock *shallow_lock,
                             const char **alternate_shallow_file,
                             const struct oid_array *extra)
 {
        struct strbuf sb = STRBUF_INIT;
        int fd;
 
-       fd = hold_lock_file_for_update(shallow_lock,
+       fd = hold_lock_file_for_update(&shallow_lock->lock,
                                       git_path_shallow(the_repository),
                                       LOCK_DIE_ON_ERROR);
        check_shallow_file_for_update(the_repository);
        if (write_shallow_commits(&sb, 0, extra)) {
                if (write_in_full(fd, sb.buf, sb.len) < 0)
                        die_errno("failed to write to %s",
-                                 get_lock_file_path(shallow_lock));
-               *alternate_shallow_file = get_lock_file_path(shallow_lock);
+                                 get_lock_file_path(&shallow_lock->lock));
+               *alternate_shallow_file = get_lock_file_path(&shallow_lock->lock);
        } else
                /*
                 * is_repository_shallow() sees empty string as "no
@@ -414,7 +414,7 @@ void advertise_shallow_grafts(int fd)
  */
 void prune_shallow(unsigned options)
 {
-       struct lock_file shallow_lock = LOCK_INIT;
+       struct shallow_lock shallow_lock = SHALLOW_LOCK_INIT;
        struct strbuf sb = STRBUF_INIT;
        unsigned flags = SEEN_ONLY;
        int fd;
@@ -428,14 +428,14 @@ void prune_shallow(unsigned options)
                strbuf_release(&sb);
                return;
        }
-       fd = hold_lock_file_for_update(&shallow_lock,
+       fd = hold_lock_file_for_update(&shallow_lock.lock,
                                       git_path_shallow(the_repository),
                                       LOCK_DIE_ON_ERROR);
        check_shallow_file_for_update(the_repository);
        if (write_shallow_commits_1(&sb, 0, NULL, flags)) {
                if (write_in_full(fd, sb.buf, sb.len) < 0)
                        die_errno("failed to write to %s",
-                                 get_lock_file_path(&shallow_lock));
+                                 get_lock_file_path(&shallow_lock.lock));
                commit_shallow_file(the_repository, &shallow_lock);
        } else {
                unlink(git_path_shallow(the_repository));

diff --git a/shallow.h b/shallow.h
index b3bf200bf9995d908e62ca7bcf72c2eab54795e1..5b4a96dcd69a2e94f4dcb2b3715e18e6373d5cac 100644 (file)
--- a/shallow.h
+++ b/shallow.h
@@ -10,10 +10,25 @@ void set_alternate_shallow_file(struct repository *r, const char *path, int over
 int register_shallow(struct repository *r, const struct object_id *oid);
 int unregister_shallow(const struct object_id *oid);
 int is_repository_shallow(struct repository *r);
+
+/*
+ * Lock for updating the $GIT_DIR/shallow file.
+ *
+ * Use `commit_shallow_file()` to commit an update, or
+ * `rollback_shallow_file()` to roll it back. In either case, any
+ * in-memory cached information about which commits are shallow will be
+ * appropriately invalidated so that future operations reflect the new
+ * state.
+ */
+struct shallow_lock {
+       struct lock_file lock;
+};
+#define SHALLOW_LOCK_INIT { LOCK_INIT }
+
 /* commit $GIT_DIR/shallow and reset stat-validity checks */
-int commit_shallow_file(struct repository *r, struct lock_file *lk);
+int commit_shallow_file(struct repository *r, struct shallow_lock *lk);
 /* rollback $GIT_DIR/shallow and reset stat-validity checks */
-void rollback_shallow_file(struct repository *r, struct lock_file *lk);
+void rollback_shallow_file(struct repository *r, struct shallow_lock *lk);
 
 struct commit_list *get_shallow_commits(struct object_array *heads,
                                        int depth, int shallow_flag, int not_shallow_flag);
@@ -22,7 +37,7 @@ struct commit_list *get_shallow_commits_by_rev_list(
 int write_shallow_commits(struct strbuf *out, int use_pack_protocol,
                          const struct oid_array *extra);
 
-void setup_alternate_shallow(struct lock_file *shallow_lock,
+void setup_alternate_shallow(struct shallow_lock *shallow_lock,
                             const char **alternate_shallow_file,
                             const struct oid_array *extra);