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

Subsystems: the rest

STALE1813d

3 messages, 3 authors, 2021-08-23 · open the first message on its own page

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

From: René Scharfe <hidden>
Date: 2021-08-21 12:52:46

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 <redacted>
---
 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

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

From: Jeff King <hidden>
Date: 2021-08-23 16:15:40

On Sat, Aug 21, 2021 at 02:52:40PM +0200, René Scharfe wrote:
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

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

From: Johannes Schindelin <hidden>
Date: 2021-08-23 20:27:54

Hi René,

On Sat, 21 Aug 2021, René Scharfe wrote:
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 <redacted>

Thanks!
Dscho
quoted hunk
Signed-off-by: René Scharfe <redacted>
---
 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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help