Open jaromil opened 4 years ago
@jaromil Feel free to simplify the internals if it aids with the bugfix and preserves portability!
Thanks for your feedbacks, knowing your interest now I'll rework the PR into something more presentable and passing tests.
@jaromil Your fix doesn't solve anything. In fact, it makes the code a lot worse in maintainability, readability and in functionality.
Let me explain. From reading your proposal and reviewing your changes I see that you try to convince us to use fstat
instead of stat
, because stat
has a limitation that fstat
doesn't have.
"No permissions are required on the file itself, but—in the case of stat(), fstatat(), and lstat() execute (search) permission is required on all of the directories in pathname that lead to the file.
The three functions stat
, fstatat
and lstat
have execute permission restriction on the directories whose path is a prefix of the path name parameter of these functions. Those restriction only apply to functions with the path name parameter. In contrast to these three functions, fstat
doesn't have a path parameter and therefore doesn't have these restrictions. Now you might think that using fstat
avoids these restrictions and is therefore a better choice, but in reality, the open
function which is being used to acquire the file descriptor fd
has the exact same limitations with the permissions.
To illustrate the limitation I have the following example. Suppose I want to get the file information from a file with the path /a/b/c/d/text.txt
. If I use stat
then the directories./
, /a
, /a/b
, /a/b/c
, /a/b/c/d
must all have the execute (search or traverse) permission in order for text.txt
to be accessible. Otherwise, this file isn't accessible. That's the limitation. Now suppose to use fstat
instead. In order to get the file information from /a/b/c/d/text.txt
via fstat
I need to open it first via the open
function. That open
function returns a file descriptor which I then supply it to fstat. Of course, that file descriptor has to be closed via close
. While fstat
doesn't require the /
, /a
, /a/b
, /a/b/c
, /a/b/c/d
directories to have execute (search or traverse) permission, the open
function does. Therefore, the execute permission is required for directories leading to text.txt
in order to acquire the file information, no matter whether stat
of fstat
is being used.
Your changes in this PR can be simplified to the following code.
#define STAT_FUNC real_stat
...
#ifndef _WIN32
static int real_stat(const char *pathname, STAT_STRUCT *buf) {
int fd = open(pathname, O_RDONLY);
if(fd<0) {
return -1;
}
int err1 = fstat(fd, buf);
int err2 = close(fd);
return !err1 && !err2 ? 0 : -1;
}
#endif
With the simplification, I created a new function called real_stat
. real_stat
internally uses fstat
in order to avoid the execute permission limitation for directories leading to pathname
. The problem with it is that open
has this limitation. Therefore real_path
has the exact same execute permission requirement for directories leading to pathname
as stat
. No matter how much this code gets turned and twisted, real_path
gets still the same limitation. The execute permission requirement for directories leading to pathname
cannot be avoided in the unix systems and actually makes perfect sense. These directories need execute (search or traverse) permission in order for the file in pathname
to be reachable. In a certain way, real_stat
is more limiting that stat
. real_stat
needs read
permission for the file because of open(pathname, O_RDONLY);
whereas stat
doesn't need any permission on the file.
In summary. Looking from a logical perspective, using fstat
instead of stat
doesn't relax any restriction. In contrary, it add restrictions, because using fstat
requires the open
function which then requires some permission such as read
. Also, the additional code adds maintenance burden.
Hi there!
Here is my fix to retrieve correct file information on POSX systems when along the path traversal a directory has no executable flag.
From
stat(2)
manpage:At the moment this is an incomplete fix: it breaks usage of
lstat(2)
on POSIX systems, due to change of the function signature passed to_file_info_()
.Kindly provide feedback on how to proceed. IMHO the usage of a function pointer passed as argument of _fileinfo is a design burden: a simple flag can be used.
ciao
p.s. for the impatient, a quick non-breaking fix is applied in the lfs version shipped in my software.