From b1dad4cfcc6fb5459e5ce34e64485234aec6e43e Mon Sep 17 00:00:00 2001 From: Sagar Vemuri Date: Wed, 26 Sep 2018 13:19:22 -0700 Subject: [PATCH] assert in PosixEnv::FileExists should be based on errno (#4427) Summary: The assert in PosixEnv::FileExists is currently based on the return value of `access` syscall. Instead it should be based on errno. Initially I wanted to remove this assert as [`access`](https://linux.die.net/man/2/access) can error out in a few other cases (like EROFS). But on thinking more it feels like the assert is doing the right thing ... its good to crash on EROFS, EFAULT, EINVAL, and other major filesystem related problems so that the user is immediately aware of the problems while testing. (I think it might be ok to crash on EIO as well, but there might be a specific reason why it was decided not to crash for EIO, and I don't have that context. So letting the letting the assert checks remain as is for now). Pull Request resolved: https://github.com/facebook/rocksdb/pull/4427 Differential Revision: D10037200 Pulled By: sagar0 fbshipit-source-id: 5cc96116a2e53cef701f444a8b5290576f311e51 --- env/env_posix.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/env/env_posix.cc b/env/env_posix.cc index 2b53fd8ab..34d49b9dc 100644 --- a/env/env_posix.cc +++ b/env/env_posix.cc @@ -520,7 +520,8 @@ class PosixEnv : public Env { return Status::OK(); } - switch (errno) { + int err = errno; + switch (err) { case EACCES: case ELOOP: case ENAMETOOLONG: @@ -528,8 +529,8 @@ class PosixEnv : public Env { case ENOTDIR: return Status::NotFound(); default: - assert(result == EIO || result == ENOMEM); - return Status::IOError("Unexpected error(" + ToString(result) + + assert(err == EIO || err == ENOMEM); + return Status::IOError("Unexpected error(" + ToString(err) + ") accessing file `" + fname + "' "); } }