From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jorge <griffin@gmx.es>, git@vger.kernel.org
Subject: [PATCH 3/4] config.c: avoid xmmap error messages
Date: Thu, 28 May 2015 03:56:15 -0400 [thread overview]
Message-ID: <20150528075614.GC23395@peff.net> (raw)
In-Reply-To: <20150528075142.GB3688@peff.net>
The config-writing code uses xmmap to map the existing
config file, which will die if the map fails. This has two
downsides:
1. The error message is not very helpful, as it lacks any
context about the file we are mapping:
$ mkdir foo
$ git config --file=foo some.key value
fatal: Out of memory? mmap failed: No such device
2. We normally do not die in this code path; instead, we'd
rather report the error and return an appropriate exit
status (which is part of the public interface
documented in git-config.1).
This patch introduces a "gentle" form of xmmap which lets us
produce our own error message. We do not want to use mmap
directly, because we would like to use the other
compatibility elements of xmmap (e.g., handling 0-length
maps portably).
The end result is:
$ git.compile config --file=foo some.key value
error: unable to mmap 'foo': No such device
$ echo $?
3
Signed-off-by: Jeff King <peff@peff.net>
---
config.c | 11 +++++++++--
git-compat-util.h | 1 +
sha1_file.c | 15 +++++++++++----
3 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/config.c b/config.c
index 6917100..e7dc155 100644
--- a/config.c
+++ b/config.c
@@ -2053,8 +2053,15 @@ int git_config_set_multivar_in_file(const char *config_filename,
fstat(in_fd, &st);
contents_sz = xsize_t(st.st_size);
- contents = xmmap(NULL, contents_sz, PROT_READ,
- MAP_PRIVATE, in_fd, 0);
+ contents = xmmap_gently(NULL, contents_sz, PROT_READ,
+ MAP_PRIVATE, in_fd, 0);
+ if (contents == MAP_FAILED) {
+ error("unable to mmap '%s': %s",
+ config_filename, strerror(errno));
+ ret = CONFIG_INVALID_FILE;
+ contents = NULL;
+ goto out_free;
+ }
close(in_fd);
if (chmod(lock->filename.buf, st.st_mode & 07777) < 0) {
diff --git a/git-compat-util.h b/git-compat-util.h
index 17584ad..0cc7ae8 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -718,6 +718,7 @@ extern char *xstrndup(const char *str, size_t len);
extern void *xrealloc(void *ptr, size_t size);
extern void *xcalloc(size_t nmemb, size_t size);
extern void *xmmap(void *start, size_t length, int prot, int flags, int fd, off_t offset);
+extern void *xmmap_gently(void *start, size_t length, int prot, int flags, int fd, off_t offset);
extern ssize_t xread(int fd, void *buf, size_t len);
extern ssize_t xwrite(int fd, const void *buf, size_t len);
extern ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
diff --git a/sha1_file.c b/sha1_file.c
index ccc6dac..73e0bc0 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -707,8 +707,8 @@ static void mmap_limit_check(size_t length)
(uintmax_t)length, (uintmax_t)limit);
}
-void *xmmap(void *start, size_t length,
- int prot, int flags, int fd, off_t offset)
+void *xmmap_gently(void *start, size_t length,
+ int prot, int flags, int fd, off_t offset)
{
void *ret;
@@ -719,12 +719,19 @@ void *xmmap(void *start, size_t length,
return NULL;
release_pack_memory(length);
ret = mmap(start, length, prot, flags, fd, offset);
- if (ret == MAP_FAILED)
- die_errno("Out of memory? mmap failed");
}
return ret;
}
+void *xmmap(void *start, size_t length,
+ int prot, int flags, int fd, off_t offset)
+{
+ void *ret = xmmap_gently(start, length, prot, flags, fd, offset);
+ if (ret == MAP_FAILED)
+ die_errno("Out of memory? mmap failed");
+ return ret;
+}
+
void close_pack_windows(struct packed_git *p)
{
while (p->windows) {
--
2.4.2.668.gc3b1ade.dirty
next prev parent reply other threads:[~2015-05-28 7:56 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-27 13:29 Bug: .gitconfig folder Jorge
2015-05-27 20:30 ` Junio C Hamano
2015-05-27 22:18 ` Jeff King
2015-05-27 22:24 ` Stefan Beller
2015-05-28 6:13 ` Jeff King
2015-05-27 22:38 ` Junio C Hamano
2015-05-28 7:51 ` Jeff King
2015-05-28 7:54 ` [PATCH 1/4] read-cache.c: drop PROT_WRITE from mmap of index Jeff King
2015-05-28 7:54 ` [PATCH 2/4] config.c: fix mmap leak when writing config Jeff King
2015-06-30 14:34 ` [PATCH] config.c: fix writing config files on Windows network shares Karsten Blees
2015-06-30 14:46 ` Torsten Bögershausen
2015-06-30 16:01 ` Jeff King
2015-06-30 14:52 ` Johannes Schindelin
2015-06-30 16:00 ` Jeff King
2015-05-28 7:56 ` Jeff King [this message]
2015-05-28 8:03 ` [PATCH 4/4] config.c: rewrite ENODEV into EISDIR when mmap fails Jeff King
2015-05-28 17:11 ` Junio C Hamano
2015-05-28 20:44 ` Jeff King
2015-05-28 21:11 ` Junio C Hamano
2015-05-28 17:06 ` Bug: .gitconfig folder Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150528075614.GC23395@peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=griffin@gmx.es \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).