git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Subject: [PATCH 14/14] verify-commit: simplify parameters to run_gpg_verify()
Date: Thu, 9 May 2019 17:32:29 -0400	[thread overview]
Message-ID: <20190509213229.GN15837@sigill.intra.peff.net> (raw)
In-Reply-To: <20190509212558.GA15438@sigill.intra.peff.net>

The buf/len parameters of run_gpg_verify() have never been used since
the function was added in d07b00b7f3 (verify-commit: scriptable commit
signature verification, 2014-06-23). Instead, check_commit_signature()
accesses the commit struct directly.

Worse, we read the whole object just to check its type and do not attach
it to the "struct commit". Meaning we end up loading the object from
disk twice for no good reason.

And to further confuse matters, our type check is comes from what we
read from disk, but we later assume that lookup_commit() will return
non-NULL. This might not be true if some other object previously
referenced the same oid as a non-commit (though this may be impossible
to trigger in practice since we don't generally parse any other objects
in this command).

Instead, let's do our type check by loading the object via
parse_object(). That will attach the buffer to the struct so it can be
used later by check_commit_signature(). And it ensures that
lookup_commit() will return something sane.

And then we can just drop the unused "buf" and "len" parameters
entirely.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/verify-commit.c | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/builtin/verify-commit.c b/builtin/verify-commit.c
index 7772c07ed7..4b9e823f8f 100644
--- a/builtin/verify-commit.c
+++ b/builtin/verify-commit.c
@@ -21,15 +21,14 @@ static const char * const verify_commit_usage[] = {
 		NULL
 };
 
-static int run_gpg_verify(const struct object_id *oid, const char *buf, unsigned long size, unsigned flags)
+static int run_gpg_verify(struct commit *commit, unsigned flags)
 {
 	struct signature_check signature_check;
 	int ret;
 
 	memset(&signature_check, 0, sizeof(signature_check));
 
-	ret = check_commit_signature(lookup_commit(the_repository, oid),
-				     &signature_check);
+	ret = check_commit_signature(commit, &signature_check);
 	print_signature_buffer(&signature_check, flags);
 
 	signature_check_clear(&signature_check);
@@ -38,26 +37,20 @@ static int run_gpg_verify(const struct object_id *oid, const char *buf, unsigned
 
 static int verify_commit(const char *name, unsigned flags)
 {
-	enum object_type type;
 	struct object_id oid;
-	char *buf;
-	unsigned long size;
-	int ret;
+	struct object *obj;
 
 	if (get_oid(name, &oid))
 		return error("commit '%s' not found.", name);
 
-	buf = read_object_file(&oid, &type, &size);
-	if (!buf)
+	obj = parse_object(the_repository, &oid);
+	if (!obj)
 		return error("%s: unable to read file.", name);
-	if (type != OBJ_COMMIT)
+	if (obj->type != OBJ_COMMIT)
 		return error("%s: cannot verify a non-commit object of type %s.",
-				name, type_name(type));
-
-	ret = run_gpg_verify(&oid, buf, size, flags);
+				name, type_name(obj->type));
 
-	free(buf);
-	return ret;
+	return run_gpg_verify((struct commit *)obj, flags);
 }
 
 static int git_verify_commit_config(const char *var, const char *value, void *cb)
-- 
2.21.0.1382.g4c6032d436

  parent reply	other threads:[~2019-05-09 21:32 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-09 21:25 [PATCH 0/14] "final" batch of unused parameter cleanups Jeff King
2019-05-09 21:27 ` [PATCH 01/14] cmd_{read,write}_tree: rename "unused" variable that is used Jeff King
2019-05-10 12:07   ` Derrick Stolee
2019-05-13  5:14   ` Junio C Hamano
2019-05-09 21:27 ` [PATCH 02/14] submodule: drop unused prefix parameter from some functions Jeff King
2019-05-09 21:28 ` [PATCH 03/14] builtin: consistently pass cmd_* prefix to parse_options Jeff King
2019-05-10 12:10   ` Derrick Stolee
2019-05-09 21:29 ` [PATCH 04/14] clone: drop dest parameter from copy_alternates() Jeff King
2019-05-09 21:29 ` [PATCH 05/14] read-cache: drop unused parameter from threaded load Jeff King
2019-05-09 21:30 ` [PATCH 06/14] wt-status: drop unused status parameter Jeff King
2019-05-09 21:30 ` [PATCH 07/14] mktree: drop unused length parameter Jeff King
2019-05-09 21:30 ` [PATCH 08/14] name-rev: drop unused parameters from is_better_name() Jeff King
2019-05-09 21:31 ` [PATCH 09/14] pack-objects: drop unused rev_info parameters Jeff King
2019-05-09 21:31 ` [PATCH 10/14] receive-pack: drop unused "commands" from prepare_shallow_update() Jeff King
2019-05-09 21:31 ` [PATCH 11/14] remove_all_fetch_refspecs(): drop unused "remote" parameter Jeff King
2019-05-09 21:32 ` [PATCH 12/14] rev-list: drop unused void pointer from finish_commit() Jeff King
2019-05-09 21:32 ` [PATCH 13/14] show-branch: drop unused parameter from show_independent() Jeff King
2019-05-09 21:32 ` Jeff King [this message]
2019-05-10 12:19   ` [PATCH 14/14] verify-commit: simplify parameters to run_gpg_verify() Derrick Stolee
2019-05-10 12:20 ` [PATCH 0/14] "final" batch of unused parameter cleanups Derrick Stolee

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=20190509213229.GN15837@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).