Re: [PATCH] [Outreachy PATCH] diff-delta: use safe integer arithmetic
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 23, 2026, 05:04 UTC
- Message-ID
- <xmqqld7paty9.fsf@gitster.g>
- In-Reply-To
- <pull.2276.git.1792727808109.gitgitgadget@gmail.com>
"Srujith A Poojary via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 5 quoted lines
> From: Srujith A Poojary <poojarysrujith@gmail.com> > > Replace manual multiplication and addition in create_delta_index() > with safe integer helpers (st_mult, st_add, st_add3) to prevent > overflows while preserving NULL returns on failure.
I do not quite understand what the last part of this explanation wants to say, i.e., "while preserving NULL returns on failure". It makes it sound as if the function would return NULL if the integer helpers detect integer overflow, but I do not think that is what actually the updated code does at all. st_add() and friends will simply die() when overflow happens, so you will not even reach malloc(), let alone return NULL when the malloc fails.
And if you reply to this review message, please do not top-post.
Thanks.
Show 42 quoted lines
>
> Signed-off-by: Srujith A Poojary <poojarysrujith@gmail.com>
> ---
> [Outreachy PATCH] diff-delta: use safe integer arithmetic
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2276%2Fpoojarysrujith%2Fc-refactor-target-name-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2276/poojarysrujith/c-refactor-target-name-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/2276
>
> diff-delta.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/diff-delta.c b/diff-delta.c
> index 7cbedeb507..45df070ab9 100644
> --- a/diff-delta.c
> +++ b/diff-delta.c
> @@ -162,8 +162,8 @@ struct delta_index * create_delta_index(const void *buf, size_t bufsize)
> hmask = hsize - 1;
>
> /* allocate lookup index */
> - memsize = sizeof(*hash) * hsize +
> - sizeof(*entry) * entries;
> + memsize = st_add(st_mult(sizeof(*hash), hsize),
> + st_mult(sizeof(*entry), entries));
> mem = malloc(memsize);
> if (!mem)
> return NULL;
> @@ -258,9 +258,9 @@ struct delta_index * create_delta_index(const void *buf, size_t bufsize)
> * Now create the packed index in array form
> * rather than linked lists.
> */
> - memsize = sizeof(*index)
> - + sizeof(*packed_hash) * (hsize+1)
> - + sizeof(*packed_entry) * entries;
> + memsize = st_add3(sizeof(*index),
> + st_mult(sizeof(*packed_hash), hsize + 1),
> + st_mult(sizeof(*packed_entry), entries));
> mem = malloc(memsize);
> if (!mem) {
> free(hash);
>
> base-commit: 47fdf2208c5b94c8cc01c106b8f7c85efb91c926