Cygwin: fhandler_virtual: move fileid to path_conv member

Commit 80f722e97c ("Cygwin: opendir(3): move ENOTDIR check into main
function") introduced a bug in fhandler_virtual handling.  While the
assertion that path_conv::check() already calls exists() and sets
FILE_ATTRIBUTE_DIRECTORY accordingly, the exists() function is called
on a fhandler_virtual object created for just this code snippet.  The
side effect of this is that the fileid member in the calling
fhandler_virtual object is not set after path_conv::check().

Move the fhandler_virtual::fileid member to path_conv::_virt_fileid
and create matching path_conv::virt_fileid() and fhandler_virtual::fileid()
methods.

Let path_conv::check() propagate the fileid set in the local
fhandler_virtual::exists() call to its own  _virt_fileid.

Use new fhandler_virtual::fileid() method throughout.

Fixes: 80f722e97c ("Cygwin: opendir(3): move ENOTDIR check into main function")
Reported-by: Bruce Jerrick <bmj001@gmail.com>
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
This commit is contained in:
Corinna Vinschen 2024-04-02 14:43:27 +02:00
parent b7f5a33200
commit a0a25849f9
8 changed files with 36 additions and 24 deletions

View file

@ -185,7 +185,7 @@ fhandler_proc::exists ()
PROC_LINK_COUNT); PROC_LINK_COUNT);
if (entry) if (entry)
{ {
fileid = entry - proc_tab; fileid () = entry - proc_tab;
return entry->type; return entry->type;
} }
return virt_none; return virt_none;
@ -377,7 +377,7 @@ fhandler_proc::open (int flags, mode_t mode)
goto out; goto out;
} }
fileid = proc_file_no; fileid () = proc_file_no;
if (!fill_filebuf ()) if (!fill_filebuf ())
{ {
res = 0; res = 0;
@ -401,9 +401,9 @@ out:
bool bool
fhandler_proc::fill_filebuf () fhandler_proc::fill_filebuf ()
{ {
if (fileid < PROC_LINK_COUNT && proc_tab[fileid].format_func) if (fileid () < PROC_LINK_COUNT && proc_tab[fileid ()].format_func)
{ {
filesize = proc_tab[fileid].format_func (NULL, filebuf); filesize = proc_tab[fileid ()].format_func (NULL, filebuf);
if (filesize > 0) if (filesize > 0)
return true; return true;
} }

View file

@ -103,12 +103,12 @@ fhandler_process::exists ()
{ {
if (!path[entry->name_len + 1]) if (!path[entry->name_len + 1])
{ {
fileid = entry - process_tab; fileid () = entry - process_tab;
return entry->type; return entry->type;
} }
if (entry->type == virt_directory) /* fd subdir only */ if (entry->type == virt_directory) /* fd subdir only */
{ {
fileid = entry - process_tab; fileid () = entry - process_tab;
if (fill_filebuf ()) if (fill_filebuf ())
return fd_type; return fd_type;
/* Check for nameless device entries. */ /* Check for nameless device entries. */
@ -200,7 +200,7 @@ DIR *
fhandler_process::opendir (int fd) fhandler_process::opendir (int fd)
{ {
DIR *dir = fhandler_virtual::opendir (fd); DIR *dir = fhandler_virtual::opendir (fd);
if (dir && process_tab[fileid].fhandler == FH_PROCESSFD) if (dir && process_tab[fileid ()].fhandler == FH_PROCESSFD)
fill_filebuf (); fill_filebuf ();
return dir; return dir;
} }
@ -215,14 +215,14 @@ int
fhandler_process::readdir (DIR *dir, dirent *de) fhandler_process::readdir (DIR *dir, dirent *de)
{ {
int res = ENMFILE; int res = ENMFILE;
if (process_tab[fileid].fhandler == FH_PROCESSFD) if (process_tab[fileid ()].fhandler == FH_PROCESSFD)
{ {
if ((size_t) dir->__d_position >= 2 + filesize / sizeof (int)) if ((size_t) dir->__d_position >= 2 + filesize / sizeof (int))
goto out; goto out;
} }
else if (dir->__d_position >= PROCESS_LINK_COUNT) else if (dir->__d_position >= PROCESS_LINK_COUNT)
goto out; goto out;
if (process_tab[fileid].fhandler == FH_PROCESSFD && dir->__d_position > 1) if (process_tab[fileid ()].fhandler == FH_PROCESSFD && dir->__d_position > 1)
{ {
int *p = (int *) filebuf; int *p = (int *) filebuf;
__small_sprintf (de->d_name, "%d", p[dir->__d_position++ - 2]); __small_sprintf (de->d_name, "%d", p[dir->__d_position++ - 2]);
@ -297,7 +297,7 @@ fhandler_process::open (int flags, mode_t mode)
goto out; goto out;
} }
fileid = entry - process_tab; fileid () = entry - process_tab;
if (!fill_filebuf ()) if (!fill_filebuf ())
{ {
res = 0; res = 0;
@ -343,15 +343,15 @@ fhandler_process::fill_filebuf ()
return false; return false;
} }
if (process_tab[fileid].format_func) if (process_tab[fileid ()].format_func)
{ {
if (process_tab[fileid].fhandler == FH_PROCESSFD) if (process_tab[fileid ()].fhandler == FH_PROCESSFD)
{ {
process_fd_t fd = { path, p , &fd_type }; process_fd_t fd = { path, p , &fd_type };
filesize = process_tab[fileid].format_func (&fd, filebuf); filesize = process_tab[fileid ()].format_func (&fd, filebuf);
} }
else else
filesize = process_tab[fileid].format_func (p, filebuf); filesize = process_tab[fileid ()].format_func (p, filebuf);
return filesize < 0 ? false : true; return filesize < 0 ? false : true;
} }
return false; return false;

View file

@ -56,7 +56,7 @@ fhandler_procnet::exists ()
{ {
if (entry->type == virt_file && !get_adapters_addresses (NULL, AF_INET6)) if (entry->type == virt_file && !get_adapters_addresses (NULL, AF_INET6))
return virt_none; return virt_none;
fileid = entry - procnet_tab; fileid () = entry - procnet_tab;
return entry->type; return entry->type;
} }
return virt_none; return virt_none;
@ -159,7 +159,7 @@ fhandler_procnet::open (int flags, mode_t mode)
goto out; goto out;
} }
fileid = entry - procnet_tab; fileid () = entry - procnet_tab;
if (!fill_filebuf ()) if (!fill_filebuf ())
{ {
res = 0; res = 0;
@ -183,9 +183,9 @@ out:
bool bool
fhandler_procnet::fill_filebuf () fhandler_procnet::fill_filebuf ()
{ {
if (procnet_tab[fileid].format_func) if (procnet_tab[fileid ()].format_func)
{ {
filesize = procnet_tab[fileid].format_func (NULL, filebuf); filesize = procnet_tab[fileid ()].format_func (NULL, filebuf);
return true; return true;
} }
return false; return false;

View file

@ -75,7 +75,7 @@ fhandler_procsysvipc::exists ()
if (cygserver_running != CYGSERVER_OK) if (cygserver_running != CYGSERVER_OK)
return virt_none; return virt_none;
} }
fileid = entry - procsysvipc_tab; fileid () = entry - procsysvipc_tab;
return entry->type; return entry->type;
} }
return virt_none; return virt_none;
@ -181,7 +181,7 @@ fhandler_procsysvipc::open (int flags, mode_t mode)
goto out; goto out;
} }
fileid = entry - procsysvipc_tab; fileid () = entry - procsysvipc_tab;
if (!fill_filebuf ()) if (!fill_filebuf ())
{ {
res = 0; res = 0;
@ -205,9 +205,9 @@ out:
bool bool
fhandler_procsysvipc::fill_filebuf () fhandler_procsysvipc::fill_filebuf ()
{ {
if (procsysvipc_tab[fileid].format_func) if (procsysvipc_tab[fileid ()].format_func)
{ {
filesize = procsysvipc_tab[fileid].format_func (NULL, filebuf); filesize = procsysvipc_tab[fileid ()].format_func (NULL, filebuf);
return true; return true;
} }
return false; return false;

View file

@ -20,7 +20,7 @@ details. */
#include <dirent.h> #include <dirent.h>
fhandler_virtual::fhandler_virtual (): fhandler_virtual::fhandler_virtual ():
fhandler_base (), filebuf (NULL), fileid (-1) fhandler_base (), filebuf (NULL)
{ {
} }

View file

@ -2932,8 +2932,8 @@ class fhandler_virtual : public fhandler_base
char *filebuf; char *filebuf;
off_t filesize; off_t filesize;
off_t position; off_t position;
int fileid; // unique within each class
bool diropen; bool diropen;
public: public:
fhandler_virtual (); fhandler_virtual ();
@ -2961,6 +2961,8 @@ class fhandler_virtual : public fhandler_base
fhandler_virtual (void *) {} fhandler_virtual (void *) {}
int &fileid () { return pc.virt_fileid (); }
virtual void copy_from (fhandler_base *x) virtual void copy_from (fhandler_base *x)
{ {
pc.free_strings (); pc.free_strings ();

View file

@ -161,6 +161,12 @@ class path_conv
const char *suffix; const char *suffix;
const char *posix_path; const char *posix_path;
path_conv_handle conv_handle; path_conv_handle conv_handle;
/* virt_fileid is used by and unique within each fhandler_virtual class.
We need it here to avoid calling the exists() method too often, in
case the derived class has a costly exists() operation.
virt_fileid is evaluated by the fhandler_virtual::exists() call in
path_conv::check and propageted to the caller's path_conv. */
int _virt_fileid;
void add_ext_from_sym (symlink_info&); void add_ext_from_sym (symlink_info&);
char *modifiable_path () {return (char *) path;} char *modifiable_path () {return (char *) path;}
@ -169,6 +175,8 @@ class path_conv
int error; int error;
device dev; device dev;
int &virt_fileid() { return _virt_fileid; }
void *serialize (HANDLE, unsigned int &) const; void *serialize (HANDLE, unsigned int &) const;
HANDLE deserialize (void *); HANDLE deserialize (void *);

View file

@ -673,6 +673,7 @@ path_conv::check (const char *src, unsigned opt,
bool add_ext = false; bool add_ext = false;
bool is_relpath; bool is_relpath;
char *tail, *path_end; char *tail, *path_end;
virt_fileid () = -1;
#if 0 #if 0
static path_conv last_path_conv; static path_conv last_path_conv;
@ -826,6 +827,7 @@ path_conv::check (const char *src, unsigned opt,
else else
{ {
file_type = fh->exists (); file_type = fh->exists ();
virt_fileid () = fh->fileid ();
if (file_type == virt_symlink if (file_type == virt_symlink
|| file_type == virt_fdsymlink) || file_type == virt_fdsymlink)
{ {