git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jorge <griffin@gmx.es>, git@vger.kernel.org
Subject: [PATCH 4/4] config.c: rewrite ENODEV into EISDIR when mmap fails
Date: Thu, 28 May 2015 04:03:01 -0400	[thread overview]
Message-ID: <20150528080300.GD23395@peff.net> (raw)
In-Reply-To: <20150528075142.GB3688@peff.net>

If we try to mmap a directory, we'll get ENODEV. This
translates to "no such device" for the user, which is not
very helpful. Since we've just fstat()'d the file, we can
easily check whether the problem was a directory to give a
better message.

Signed-off-by: Jeff King <peff@peff.net>
---
It feels a bit wrong to put this magic conversion here, and not in
xmmap. But of course xmmap does not have the stat information.

Which makes me wonder if we should provide an interface that will take
the whole "struct stat" rather than just the size. That's less flexible,
but in most cases, we're mapping the whole file (the packfiles are the
big exception, where we use a window).

We could also potentially drop some of the useless options. As of patch
1, all of our calls are PROT_READ. They must all be MAP_PRIVATE, or our
pread compatibility wrapper will fail, and we never use other flags. We
never request a specific address. And in a whole-file remap, the offset
will always be 0. So something like:

  void *xmmap_file(int fd, struct stat *st);

would probably work. We could even do the fstat() on behalf of the
caller, though they need to know the length themselves. Maybe:

  void *xmmap_file(int fd, size_t *len);

I dunno if it is worth it or not.

 config.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/config.c b/config.c
index e7dc155..29fa012 100644
--- a/config.c
+++ b/config.c
@@ -2056,6 +2056,8 @@ int git_config_set_multivar_in_file(const char *config_filename,
 		contents = xmmap_gently(NULL, contents_sz, PROT_READ,
 					MAP_PRIVATE, in_fd, 0);
 		if (contents == MAP_FAILED) {
+			if (errno == ENODEV && S_ISDIR(st.st_mode))
+				errno = EISDIR;
 			error("unable to mmap '%s': %s",
 			      config_filename, strerror(errno));
 			ret = CONFIG_INVALID_FILE;
-- 
2.4.2.668.gc3b1ade.dirty

  parent reply	other threads:[~2015-05-28  8:03 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         ` [PATCH 3/4] config.c: avoid xmmap error messages Jeff King
2015-05-28  8:03         ` Jeff King [this message]
2015-05-28 17:11           ` [PATCH 4/4] config.c: rewrite ENODEV into EISDIR when mmap fails 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=20150528080300.GD23395@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).