All of lore.kernel.org
 help / color / mirror / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, jonathantanmy@google.com, jrnieder@gmail.com
Subject: [PATCH 5/5] shallow: use struct 'shallow_lock' for additional safety
Date: Wed, 29 Apr 2020 16:39:46 -0600	[thread overview]
Message-ID: <c0dc5024a9b368dfbca99b81bc843f66d725f3d7.1588199705.git.me@ttaylorr.com> (raw)
In-Reply-To: <cover.1588199705.git.me@ttaylorr.com>

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>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 builtin/receive-pack.c |  2 +-
 fetch-pack.c           |  2 +-
 shallow.c              | 22 +++++++++++-----------
 shallow.h              | 16 +++++++++++++---
 4 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 652661fa99..10fd2c52ec 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -855,7 +855,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 401d028e41..27d2d08cc0 100644
--- 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 76e00893fe..1acf8ce95b 100644
--- 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->lk);
 	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->lk);
 	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->lk,
 				       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->lk));
+		*alternate_shallow_file = get_lock_file_path(&shallow_lock->lk);
 	} 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.lk,
 				       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.lk));
 		commit_shallow_file(the_repository, &shallow_lock);
 	} else {
 		unlink(git_path_shallow(the_repository));
diff --git a/shallow.h b/shallow.h
index 08e1bc4fd0..d12096fbc4 100644
--- a/shallow.h
+++ b/shallow.h
@@ -10,12 +10,22 @@ 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);
+
+/*
+ * shallow_lock is a thin wrapper around 'struct lock_file' in order to restrict
+ * which locks can be used with '{commit,rollback}_shallow_file()'.
+ */
+struct shallow_lock {
+	struct lock_file lk;
+};
+#define SHALLOW_LOCK_INIT { LOCK_INIT }
+
 /*
  * {commit,rollback}_shallow_file commits or performs a rollback to the
  * '.git/shallow' file, respectively, and resets stat-validity checks.
  */
-int commit_shallow_file(struct repository *r, struct lock_file *lk);
-void rollback_shallow_file(struct repository *r, struct lock_file *lk);
+int commit_shallow_file(struct repository *r, struct shallow_lock *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);
@@ -24,7 +34,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);
 
-- 
2.26.0.113.ge9739cdccc

  parent reply	other threads:[~2020-04-29 22:39 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-29 22:39 [PATCH 0/5] shallow: extract a header file Taylor Blau
2020-04-29 22:39 ` [PATCH 1/5] commit: make 'commit_graft_pos' non-static Taylor Blau
2020-04-30  3:26   ` Jonathan Nieder
2020-04-30 19:22     ` Taylor Blau
2020-04-29 22:39 ` [PATCH 2/5] shallow: extract a header file for shallow-related functions Taylor Blau
2020-04-29 22:48   ` Eric Sunshine
2020-04-30  0:21   ` Jonathan Tan
2020-04-30  0:59     ` Taylor Blau
2020-04-30 18:23       ` Jonathan Tan
2020-04-30  3:19   ` Jonathan Nieder
2020-04-29 22:39 ` [PATCH 3/5] commit: move 'unregister_shallow' to 'shallow.h' Taylor Blau
2020-04-30  3:13   ` Jonathan Nieder
2020-04-30 19:29     ` Taylor Blau
2020-04-29 22:39 ` [PATCH 4/5] shallow.h: document '{commit,rollback}_shallow_file' Taylor Blau
2020-04-29 22:53   ` Eric Sunshine
2020-04-29 22:39 ` Taylor Blau [this message]
2020-04-29 23:03   ` [PATCH 5/5] shallow: use struct 'shallow_lock' for additional safety Eric Sunshine
2020-04-29 23:51     ` Taylor Blau
2020-04-30  0:30   ` Jonathan Tan
2020-04-30  3:11   ` Jonathan Nieder
2020-04-30  5:32     ` Eric Sunshine
2020-04-30 19:32       ` Taylor Blau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c0dc5024a9b368dfbca99b81bc843f66d725f3d7.1588199705.git.me@ttaylorr.com \
    --to=me@ttaylorr.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jonathantanmy@google.com \
    --cc=jrnieder@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.