Hi Ævar and René
On 15/04/2022 14:53, Ævar Arnfjörð Bjarmason wrote:
On Fri, Apr 15 2022, René Scharfe wrote:
Am 15.04.22 um 12:21 schrieb Ævar Arnfjörð Bjarmason:
Return NULL instead of possibly feeding a NULL to memset() in
reftable_calloc(). This issue was noted by GCC 12's -fanalyzer:
reftable/publicbasics.c: In function ‘reftable_calloc’:
reftable/publicbasics.c:43:9: error: use of possibly-NULL ‘p’ where non-null expected [CWE-690] [-Werror=analyzer-possible-null-argument]
43 | memset(p, 0, sz);
| ^~~~~~~~~~~~~~~~
[...]
This bug has been with us ever since this code was added in
ef8a6c62687 (reftable: utility functions, 2021-10-07).
Not sure it's a bug, or limited to this specific location. AFAICS it's
more a general lack of handling of allocation failures in the reftable
code. Perhaps it was a conscious decision to let the system deal with
them via segfaults?
I think it's just buggy, it tries to deal with malloc returning NULL in
other contexts.
Does it? I just quickly scanned the output of
git grep -e 'reftable_[mc]alloc' -e 'reftable_realloc' -A2 origin/master
and I didn't see a single NULL check
When the code is used by Git eventually it should use xmalloc and
xrealloc instead of calling malloc(3) and realloc(3) directly, to
handle allocation failures explicitly, and to support GIT_ALLOC_LIMIT.
This will calm down the analyzer and extend the safety to the rest of
the reftable code, not just this memset call.
Yeah, its whole custom malloc handling should go away.
Isn't it there so the same code can be used by libgit2 and other things
that let the caller specify a custom allocator?
Best Wishes
Phillip
