threads / patch / 56342

patchcompat: let git_mmap use malloc(3) directly

Subject: [PATCH] compat: let git_mmap use malloc(3) directly

## tl;dr

3 messages between Aug 21, 2021 and Aug 23, 2021. Diffs are folded; open one to read it.

replies: 2people: 3as markdown or json

René Scharfe· Aug 21, 2021, 12:52 UTC · lore

xmalloc() dies on error, allows zero-sized allocations and enforces GIT_ALLOC_LIMIT for testing. Our mmap replacement doesn't need any of that. Let's cut out the wrapper, reject zero-sized requests as required by POSIX and use malloc(3) directly. Allocation errors were needlessly handled by git_mmap() before; this code becomes reachable now.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 compat/mmap.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
Show changes to compat/mmap.c +6 −2
diff --git a/compat/mmap.c b/compat/mmap.c
index 14d31010df..8d6c02d4bc 100644
--- a/compat/mmap.c
+++ b/compat/mmap.c
@@ -7,7 +7,12 @@ void *git_mmap(void *start, size_t length, int prot, int flags, int fd, off_t of
 	if (start != NULL || flags != MAP_PRIVATE || prot != PROT_READ)
 		die("Invalid usage of mmap when built with NO_MMAP");

-	start = xmalloc(length);
+	if (length == 0) {
+		errno = EINVAL;
+		return MAP_FAILED;
+	}
+
+	start = malloc(length);
 	if (start == NULL) {
 		errno = ENOMEM;
 		return MAP_FAILED;
--
2.33.0
Jeff King· Aug 23, 2021, 16:15 UTC · re: René Scharfe · lore

Re: [PATCH] compat: let git_mmap use malloc(3) directly

On Sat, Aug 21, 2021 at 02:52:40PM +0200, René Scharfe wrote:
Show 5 quoted lines
> xmalloc() dies on error, allows zero-sized allocations and enforces
> GIT_ALLOC_LIMIT for testing.  Our mmap replacement doesn't need any of
> that.  Let's cut out the wrapper, reject zero-sized requests as required
> by POSIX and use malloc(3) directly.  Allocation errors were needlessly
> handled by git_mmap() before; this code becomes reachable now.

Thanks, this is a good idea. As you note, the function was already expecting a possible NULL return. And I think in general it's good for our compat/ wrappers to stay as close to the OS-level functions they're emulating as possible.

-Peff
Johannes Schindelin· Aug 23, 2021, 20:27 UTC · re: René Scharfe · lore

Re: [PATCH] compat: let git_mmap use malloc(3) directly

Hi René,
On Sat, 21 Aug 2021, René Scharfe wrote:
Show 5 quoted lines
> xmalloc() dies on error, allows zero-sized allocations and enforces
> GIT_ALLOC_LIMIT for testing.  Our mmap replacement doesn't need any of
> that.  Let's cut out the wrapper, reject zero-sized requests as required
> by POSIX and use malloc(3) directly.  Allocation errors were needlessly
> handled by git_mmap() before; this code becomes reachable now.
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Thanks! Dscho

Show 27 quoted lines
>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
>  compat/mmap.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/compat/mmap.c b/compat/mmap.c
> index 14d31010df..8d6c02d4bc 100644
> --- a/compat/mmap.c
> +++ b/compat/mmap.c
> @@ -7,7 +7,12 @@ void *git_mmap(void *start, size_t length, int prot, int flags, int fd, off_t of
>  	if (start != NULL || flags != MAP_PRIVATE || prot != PROT_READ)
>  		die("Invalid usage of mmap when built with NO_MMAP");
>
> -	start = xmalloc(length);
> +	if (length == 0) {
> +		errno = EINVAL;
> +		return MAP_FAILED;
> +	}
> +
> +	start = malloc(length);
>  	if (start == NULL) {
>  		errno = ENOMEM;
>  		return MAP_FAILED;
> --
> 2.33.0
>

← back to recent threads