Jeff King <peff@xxxxxxxx> writes:
> 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@xxxxxxxx>
> ---
> 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.
> ...
> 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;
I think this patch places the "magic" at the right place, but I
would have preferred to see something more like this:
if (contents == MAP_FAILED) {
if (errno == ENODEV && S_ISDIR(st.st_mode))
error("unable to mmap a directory '%s',
config_filename);
else
error("unable to mmap '%s': %s",
config_filename, strerror(errno));
ret = CONFIG_INVALID_FILE;
But that is a very minor preference. I am OK with relying on our
knowledge that strerror(EISDIR) would give something that says "the
thing is a directory which is not appropriate for the operation", as
nobody after that strerror() refers to 'errno' in this codepath.
Thanks. The patches were a pleasant read.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html