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;
