All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: "René Scharfe" <l.s.r@web.de>,
	phillip.wood@dunelm.org.uk, Cheng <prophecheng@stu.pku.edu.cn>,
	git@vger.kernel.org
Subject: Re: [PATCH 1/5] describe: pass oid struct by const pointer
Date: Mon, 18 Aug 2025 14:05:36 -0700	[thread overview]
Message-ID: <xmqqfrdoibmn.fsf@gitster.g> (raw)
In-Reply-To: <20250818205929.GA1024556@coredump.intra.peff.net> (Jeff King's message of "Mon, 18 Aug 2025 16:59:29 -0400")

Jeff King <peff@peff.net> writes:

> We pass a "struct object_id" to describe_blob() by value. This isn't
> wrong, as an oid is composed only of copy-able values. But it's unusual;
> typically we pass structs by const pointer, including object_ids. Let's
> do so.
>
> It similarly makes sense for us to hold that pointer in the callback
> data (rather than yet another copy of the oid).
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> Not strictly related, but I noticed while in the area and remembered a
> recent discussion in this direction.

Thanks for making this part of the code less odd.

>  builtin/describe.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/describe.c b/builtin/describe.c
> index d7dd8139de..383d3e6b9a 100644
> --- a/builtin/describe.c
> +++ b/builtin/describe.c
> @@ -490,7 +490,7 @@ static void describe_commit(struct object_id *oid, struct strbuf *dst)
>  
>  struct process_commit_data {
>  	struct object_id current_commit;
> -	struct object_id looking_for;
> +	const struct object_id *looking_for;
>  	struct strbuf *dst;
>  	struct rev_info *revs;
>  };
> @@ -505,7 +505,7 @@ static void process_object(struct object *obj, const char *path, void *data)
>  {
>  	struct process_commit_data *pcd = data;
>  
> -	if (oideq(&pcd->looking_for, &obj->oid) && !pcd->dst->len) {
> +	if (oideq(pcd->looking_for, &obj->oid) && !pcd->dst->len) {
>  		reset_revision_walk();
>  		describe_commit(&pcd->current_commit, pcd->dst);
>  		strbuf_addf(pcd->dst, ":%s", path);
> @@ -514,7 +514,7 @@ static void process_object(struct object *obj, const char *path, void *data)
>  	}
>  }
>  
> -static void describe_blob(struct object_id oid, struct strbuf *dst)
> +static void describe_blob(const struct object_id *oid, struct strbuf *dst)
>  {
>  	struct rev_info revs;
>  	struct strvec args = STRVEC_INIT;
> @@ -554,7 +554,7 @@ static void describe(const char *arg, int last_one)
>  		describe_commit(&oid, &sb);
>  	else if (odb_read_object_info(the_repository->objects,
>  				      &oid, NULL) == OBJ_BLOB)
> -		describe_blob(oid, &sb);
> +		describe_blob(&oid, &sb);
>  	else
>  		die(_("%s is neither a commit nor blob"), arg);

  reply	other threads:[~2025-08-18 21:05 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-13  0:23 Potential Null Pointer Dereference detected by static analysis tool Cheng
2025-08-13 13:19 ` Phillip Wood
2025-08-14 23:26   ` Jeff King
2025-08-15 15:49     ` Phillip Wood
2025-08-17  9:27     ` René Scharfe
2025-08-18  4:48       ` Jeff King
2025-08-18  5:05         ` Jeff King
2025-08-18 19:56           ` René Scharfe
2025-08-18 20:21             ` Jeff King
2025-08-18 20:56               ` Jeff King
2025-08-18 20:58               ` [PATCH 0/5] fix segfault and other oddities describing blobs Jeff King
2025-08-18 20:59                 ` [PATCH 1/5] describe: pass oid struct by const pointer Jeff King
2025-08-18 21:05                   ` Junio C Hamano [this message]
2025-08-18 21:01                 ` [PATCH 2/5] describe: error if blob not found Jeff King
2025-08-18 21:12                   ` Junio C Hamano
2025-08-19  8:05                     ` Patrick Steinhardt
2025-08-19 18:32                   ` René Scharfe
2025-08-18 21:01                 ` [PATCH 3/5] describe: catch unborn branch in describe_blob() Jeff King
2025-08-18 21:19                   ` Junio C Hamano
2025-08-18 23:07                     ` Jeff King
2025-08-18 21:03                 ` [PATCH 4/5] describe: handle blob traversal with no commits Jeff King
2025-08-19  8:05                   ` Patrick Steinhardt
2025-08-19 16:59                     ` Jeff King
2025-08-20  4:34                       ` Patrick Steinhardt
2025-08-20  6:30                         ` [replacement PATCH " Jeff King
2025-08-18 21:04                 ` [PATCH 5/5] describe: pass commit to describe_commit() Jeff King
2025-08-19  8:05                   ` Patrick Steinhardt
2025-08-19 17:02                     ` Jeff King

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=xmqqfrdoibmn.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=l.s.r@web.de \
    --cc=peff@peff.net \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=prophecheng@stu.pku.edu.cn \
    /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.