From: Jeff King <peff@peff.net>
To: "SZEDER Gábor" <szeder.dev@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH] commit-slab: clarify slabname##_peek()'s return value
Date: Tue, 10 Mar 2020 14:07:48 -0400 [thread overview]
Message-ID: <20200310180748.GC549010@coredump.intra.peff.net> (raw)
In-Reply-To: <20200310175446.GB549010@coredump.intra.peff.net>
On Tue, Mar 10, 2020 at 01:54:46PM -0400, Jeff King wrote:
> I also wondered if we could make life easier for the caller by
> collapsing these cases. I.e., always returning a zero-initialized value,
> and never NULL.
> [...]
> But that would get a bit awkward, because peek() returns a pointer, not
> a value (as it should, because the type we're storing may be a compound
> type, which we generally avoid passing or returning by value). So we'd
> actually need to return a pointer to a zero-initialized dummy value. Not
> impossible, but getting a bit odd.
I was a little curious how it would look. Code-wise it's not too bad,
but if anybody ever wrote to the dummy entry, that would cause confusing
bugs. Possibly peek() could return a pointer-to-const.
The code savings in the callers are not all that much (I didn't convert
all sites, but you can see that it just saves a few lines in each case).
So it's probably not worth the churn, but if anybody is really excited
about it, I can polish it into a real patch.
diff --git a/blame.c b/blame.c
index 29770e5c81..3ae42a1edd 100644
--- a/blame.c
+++ b/blame.c
@@ -15,11 +15,7 @@ static struct blame_suspects blame_suspects;
struct blame_origin *get_blame_suspects(struct commit *commit)
{
- struct blame_origin **result;
-
- result = blame_suspects_peek(&blame_suspects, commit);
-
- return result ? *result : NULL;
+ return *blame_suspects_peek(&blame_suspects, commit);
}
static void set_blame_suspects(struct commit *commit, struct blame_origin *origin)
diff --git a/commit-slab-decl.h b/commit-slab-decl.h
index adc7b46c83..4b951e7b2f 100644
--- a/commit-slab-decl.h
+++ b/commit-slab-decl.h
@@ -13,6 +13,7 @@ struct slabname { \
unsigned stride; \
unsigned slab_count; \
elemtype **slab; \
+ elemtype dummy; \
}
/*
diff --git a/commit-slab-impl.h b/commit-slab-impl.h
index 5c0eb91a5d..505c21599a 100644
--- a/commit-slab-impl.h
+++ b/commit-slab-impl.h
@@ -22,6 +22,7 @@ scope void init_ ##slabname## _with_stride(struct slabname *s, \
s->slab_size = COMMIT_SLAB_SIZE / elem_size; \
s->slab_count = 0; \
s->slab = NULL; \
+ memset(&s->dummy, 0, sizeof(elemtype)); \
} \
\
scope void init_ ##slabname(struct slabname *s) \
@@ -50,15 +51,15 @@ scope elemtype *slabname## _at_peek(struct slabname *s, \
if (s->slab_count <= nth_slab) { \
unsigned int i; \
if (!add_if_missing) \
- return NULL; \
+ return &s->dummy; \
REALLOC_ARRAY(s->slab, nth_slab + 1); \
for (i = s->slab_count; i <= nth_slab; i++) \
s->slab[i] = NULL; \
s->slab_count = nth_slab + 1; \
} \
if (!s->slab[nth_slab]) { \
if (!add_if_missing) \
- return NULL; \
+ return &s->dummy; \
s->slab[nth_slab] = xcalloc(s->slab_size, \
sizeof(**s->slab) * s->stride); \
} \
diff --git a/commit.c b/commit.c
index a6cfa41a4e..49d86435a5 100644
--- a/commit.c
+++ b/commit.c
@@ -289,11 +289,6 @@ const void *get_cached_commit_buffer(struct repository *r, const struct commit *
{
struct commit_buffer *v = buffer_slab_peek(
r->parsed_objects->buffer_slab, commit);
- if (!v) {
- if (sizep)
- *sizep = 0;
- return NULL;
- }
if (sizep)
*sizep = v->size;
return v->buffer;
next prev parent reply other threads:[~2020-03-10 18:07 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-10 15:30 [PATCH] commit-slab: clarify slabname##_peek()'s return value SZEDER Gábor
2020-03-10 17:54 ` Jeff King
2020-03-10 18:07 ` Jeff King [this message]
2020-03-11 16:07 ` Jeff King
2020-03-10 18:19 ` Junio C Hamano
2020-03-10 18:26 ` Junio C Hamano
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200310180748.GC549010@coredump.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=szeder.dev@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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).