Cygwin: fhandler_virtual: move fileid to path_conv member

Commit 80f722e97cf7 ("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: 80f722e97cf7 ("Cygwin: opendir(3): move ENOTDIR check into main function")
Reported-by: Bruce Jerrick <bmj001@gmail.com>
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>

8 files changed, 36 insertions, 24 deletions

diff --git a/winsup/cygwin/fhandler/proc.cc b/winsup/cygwin/fhandler/proc.cc
index a508d4b..7629b54 100644
--- a/winsup/cygwin/fhandler/proc.cc
+++ b/winsup/cygwin/fhandler/proc.cc
@@ -185,7 +185,7 @@ fhandler_proc::exists ()
 				       PROC_LINK_COUNT);
   if (entry)
     {
-      fileid = entry - proc_tab;
+      fileid () = entry - proc_tab;
       return entry->type;
     }
   return virt_none;
@@ -377,7 +377,7 @@ fhandler_proc::open (int flags, mode_t mode)
       goto out;
     }
 
-  fileid = proc_file_no;
+  fileid () = proc_file_no;
   if (!fill_filebuf ())
     {
       res = 0;
@@ -401,9 +401,9 @@ out:
 bool
 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)
 	return true;
     }
diff --git a/winsup/cygwin/fhandler/process.cc b/winsup/cygwin/fhandler/process.cc
index 1e5e83b..20169d2 100644
--- a/winsup/cygwin/fhandler/process.cc
+++ b/winsup/cygwin/fhandler/process.cc
@@ -103,12 +103,12 @@ fhandler_process::exists ()
     {
       if (!path[entry->name_len + 1])
 	{
-	  fileid = entry - process_tab;
+	  fileid () = entry - process_tab;
 	  return entry->type;
 	}
       if (entry->type == virt_directory)	/* fd subdir only */
 	{
-	  fileid = entry - process_tab;
+	  fileid () = entry - process_tab;
 	  if (fill_filebuf ())
 	    return fd_type;
 	  /* Check for nameless device entries. */
@@ -200,7 +200,7 @@ DIR *
 fhandler_process::opendir (int 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 ();
   return dir;
 }
@@ -215,14 +215,14 @@ int
 fhandler_process::readdir (DIR *dir, dirent *de)
 {
   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))
 	goto out;
     }
   else if (dir->__d_position >= PROCESS_LINK_COUNT)
     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;
       __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;
     }
 
-  fileid = entry - process_tab;
+  fileid () = entry - process_tab;
   if (!fill_filebuf ())
 	{
 	  res = 0;
@@ -343,15 +343,15 @@ fhandler_process::fill_filebuf ()
       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 };
-	  filesize = process_tab[fileid].format_func (&fd, filebuf);
+	  filesize = process_tab[fileid ()].format_func (&fd, filebuf);
 	}
       else
-	filesize = process_tab[fileid].format_func (p, filebuf);
+	filesize = process_tab[fileid ()].format_func (p, filebuf);
       return filesize < 0 ? false : true;
     }
   return false;
diff --git a/winsup/cygwin/fhandler/procnet.cc b/winsup/cygwin/fhandler/procnet.cc
index d512887..112aee8 100644
--- a/winsup/cygwin/fhandler/procnet.cc
+++ b/winsup/cygwin/fhandler/procnet.cc
@@ -56,7 +56,7 @@ fhandler_procnet::exists ()
     {
       if (entry->type == virt_file && !get_adapters_addresses (NULL, AF_INET6))
 	return virt_none;
-      fileid = entry - procnet_tab;
+      fileid () = entry - procnet_tab;
       return entry->type;
     }
   return virt_none;
@@ -159,7 +159,7 @@ fhandler_procnet::open (int flags, mode_t mode)
       goto out;
     }
 
-  fileid = entry - procnet_tab;
+  fileid () = entry - procnet_tab;
   if (!fill_filebuf ())
 	{
 	  res = 0;
@@ -183,9 +183,9 @@ out:
 bool
 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 false;
diff --git a/winsup/cygwin/fhandler/procsysvipc.cc b/winsup/cygwin/fhandler/procsysvipc.cc
index 453d3b4..b322c46 100644
--- a/winsup/cygwin/fhandler/procsysvipc.cc
+++ b/winsup/cygwin/fhandler/procsysvipc.cc
@@ -75,7 +75,7 @@ fhandler_procsysvipc::exists ()
 	  if (cygserver_running != CYGSERVER_OK)
 	    return virt_none;
 	}
-	  fileid = entry - procsysvipc_tab;
+	  fileid () = entry - procsysvipc_tab;
 	  return entry->type;
 	}
   return virt_none;
@@ -181,7 +181,7 @@ fhandler_procsysvipc::open (int flags, mode_t mode)
       goto out;
     }
 
-  fileid = entry - procsysvipc_tab;
+  fileid () = entry - procsysvipc_tab;
   if (!fill_filebuf ())
 	{
 	  res = 0;
@@ -205,9 +205,9 @@ out:
 bool
 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 false;
diff --git a/winsup/cygwin/fhandler/virtual.cc b/winsup/cygwin/fhandler/virtual.cc
index 4a0d294..90653fb 100644
--- a/winsup/cygwin/fhandler/virtual.cc
+++ b/winsup/cygwin/fhandler/virtual.cc
@@ -20,7 +20,7 @@ details. */
 #include <dirent.h>
 
 fhandler_virtual::fhandler_virtual ():
-  fhandler_base (), filebuf (NULL), fileid (-1)
+  fhandler_base (), filebuf (NULL)
 {
 }
 
diff --git a/winsup/cygwin/local_includes/fhandler.h b/winsup/cygwin/local_includes/fhandler.h
index bd684a6..978d3e5 100644
--- a/winsup/cygwin/local_includes/fhandler.h
+++ b/winsup/cygwin/local_includes/fhandler.h
@@ -2932,8 +2932,8 @@ class fhandler_virtual : public fhandler_base
   char *filebuf;
   off_t filesize;
   off_t position;
-  int fileid; // unique within each class
   bool diropen;
+
  public:
 
   fhandler_virtual ();
@@ -2961,6 +2961,8 @@ class fhandler_virtual : public fhandler_base
 
   fhandler_virtual (void *) {}
 
+  int &fileid () { return pc.virt_fileid (); }
+
   virtual void copy_from (fhandler_base *x)
   {
     pc.free_strings ();
diff --git a/winsup/cygwin/local_includes/path.h b/winsup/cygwin/local_includes/path.h
index cb8c4ca..3dd21d9 100644
--- a/winsup/cygwin/local_includes/path.h
+++ b/winsup/cygwin/local_includes/path.h
@@ -161,6 +161,12 @@ class path_conv
   const char *suffix;
   const char *posix_path;
   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&);
   char *modifiable_path () {return (char *) path;}
@@ -169,6 +175,8 @@ class path_conv
   int error;
   device dev;
 
+  int &virt_fileid() { return _virt_fileid; }
+
   void *serialize (HANDLE, unsigned int &) const;
   HANDLE deserialize (void *);
 
diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index ddea4b3..fa5ceea 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -673,6 +673,7 @@ path_conv::check (const char *src, unsigned opt,
   bool add_ext = false;
   bool is_relpath;
   char *tail, *path_end;
+  virt_fileid () = -1;
 
 #if 0
   static path_conv last_path_conv;
@@ -826,6 +827,7 @@ path_conv::check (const char *src, unsigned opt,
 		  else
 		    {
 		      file_type = fh->exists ();
+		      virt_fileid () = fh->fileid ();
 		      if (file_type == virt_symlink
 			  || file_type == virt_fdsymlink)
 			{