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
main
Sagar Vemuri 6 years ago committed by Facebook Github Bot
parent d56070d875
commit b1dad4cfcc
  1. 7
      env/env_posix.cc

7
env/env_posix.cc vendored

@ -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 + "' ");
}
}

Loading…
Cancel
Save