From e35f9824159bba94eecdf22d198799701ed60940 Mon Sep 17 00:00:00 2001 From: Pavel Roskin Date: Fri, 29 Jul 2005 10:49:14 -0400 Subject: [PATCH] [PATCH] mmap error handling I have reviewed all occurrences of mmap() in git and fixed three types of errors/defects: 1) The result is not checked. 2) The file descriptor is closed if mmap() succeeds, but not when it fails. 3) Various casts applied to -1 are used instead of MAP_FAILED, which is specifically defined to check mmap() return value. [jc: This is a second round of Pavel's patch. He fixed up the problem that close() potentially clobbering the errno from mmap, which the first round had.] Signed-off-by: Pavel Roskin Signed-off-by: Junio C Hamano --- diff.c | 4 +++- diffcore-order.c | 2 +- local-pull.c | 2 +- read-cache.c | 4 ++-- rev-cache.c | 6 ++---- sha1_file.c | 4 ++-- test-delta.c | 2 ++ tools/mailsplit.c | 3 ++- 8 files changed, 15 insertions(+), 12 deletions(-) diff --git a/diff.c b/diff.c index 4a4b62191..361d7c09d 100644 --- a/diff.c +++ b/diff.c @@ -377,8 +377,10 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only) if (fd < 0) goto err_empty; s->data = mmap(NULL, s->size, PROT_READ, MAP_PRIVATE, fd, 0); - s->should_munmap = 1; close(fd); + if (s->data == MAP_FAILED) + goto err_empty; + s->should_munmap = 1; } else { char type[20]; diff --git a/diffcore-order.c b/diffcore-order.c index a03862c1c..b38122361 100644 --- a/diffcore-order.c +++ b/diffcore-order.c @@ -28,7 +28,7 @@ static void prepare_order(const char *orderfile) } map = mmap(NULL, st.st_size, PROT_READ|PROT_WRITE, MAP_PRIVATE, fd, 0); close(fd); - if (-1 == (int)(long)map) + if (map == MAP_FAILED) return; endp = map + st.st_size; for (pass = 0; pass < 2; pass++) { diff --git a/local-pull.c b/local-pull.c index 2f06fbee8..908e18750 100644 --- a/local-pull.c +++ b/local-pull.c @@ -54,7 +54,7 @@ int fetch(unsigned char *sha1) } map = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, ifd, 0); close(ifd); - if (-1 == (int)(long)map) { + if (map == MAP_FAILED) { fprintf(stderr, "cannot mmap %s\n", filename); return -1; } diff --git a/read-cache.c b/read-cache.c index f448ab17e..5820f18d9 100644 --- a/read-cache.c +++ b/read-cache.c @@ -392,7 +392,7 @@ int read_cache(void) return (errno == ENOENT) ? 0 : error("open failed"); size = 0; // avoid gcc warning - map = (void *)-1; + map = MAP_FAILED; if (!fstat(fd, &st)) { size = st.st_size; errno = EINVAL; @@ -400,7 +400,7 @@ int read_cache(void) map = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0); } close(fd); - if (-1 == (int)(long)map) + if (map == MAP_FAILED) return error("mmap failed"); hdr = map; diff --git a/rev-cache.c b/rev-cache.c index ea65274ed..f908ce7a3 100644 --- a/rev-cache.c +++ b/rev-cache.c @@ -212,11 +212,9 @@ int read_rev_cache(const char *path, FILE *dumpfile, int dry_run) return -1; } map = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0); - if (map == MAP_FAILED) { - close(fd); - return -1; - } close(fd); + if (map == MAP_FAILED) + return -1; memset(last_sha1, 0, 20); ofs = 0; diff --git a/sha1_file.c b/sha1_file.c index 5ec5598d7..eba5a36f2 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -518,7 +518,7 @@ static void *map_sha1_file_internal(const unsigned char *sha1, } map = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0); close(fd); - if (-1 == (int)(long)map) + if (map == MAP_FAILED) return NULL; *size = st.st_size; return map; @@ -1363,7 +1363,7 @@ int index_fd(unsigned char *sha1, int fd, struct stat *st, int write_object, con if (size) buf = mmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0); close(fd); - if ((int)(long)buf == -1) + if (buf == MAP_FAILED) return -1; if (!type) diff --git a/test-delta.c b/test-delta.c index 37ef86b28..e5d31ca2e 100644 --- a/test-delta.c +++ b/test-delta.c @@ -41,6 +41,7 @@ int main(int argc, char *argv[]) from_buf = mmap(NULL, from_size, PROT_READ, MAP_PRIVATE, fd, 0); if (from_buf == MAP_FAILED) { perror(argv[2]); + close(fd); return 1; } close(fd); @@ -54,6 +55,7 @@ int main(int argc, char *argv[]) data_buf = mmap(NULL, data_size, PROT_READ, MAP_PRIVATE, fd, 0); if (data_buf == MAP_FAILED) { perror(argv[3]); + close(fd); return 1; } close(fd); diff --git a/tools/mailsplit.c b/tools/mailsplit.c index 9379fbc5e..7b712081c 100644 --- a/tools/mailsplit.c +++ b/tools/mailsplit.c @@ -116,8 +116,9 @@ int main(int argc, char **argv) } size = st.st_size; map = mmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0); - if (-1 == (int)(long)map) { + if (map == MAP_FAILED) { perror("mmap"); + close(fd); exit(1); } close(fd); -- 2.30.2