public inbox for cygwin-cvs@sourceware.org
help / color / mirror / Atom feed
* [newlib-cygwin/main] Cygwin: fhandler_virtual: move fileid to path_conv member
@ 2024-04-02 13:11 Corinna Vinschen
  0 siblings, 0 replies; only message in thread
From: Corinna Vinschen @ 2024-04-02 13:11 UTC (permalink / raw)
  To: cygwin-cvs

https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=a0a25849f9dda5cd3f2963bcc05563ab797c1e42

commit a0a25849f9dda5cd3f2963bcc05563ab797c1e42
Author:     Corinna Vinschen <corinna@vinschen.de>
AuthorDate: Tue Apr 2 14:43:27 2024 +0200
Commit:     Corinna Vinschen <corinna@vinschen.de>
CommitDate: Tue Apr 2 14:43:27 2024 +0200

    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>

Diff:
---
 winsup/cygwin/fhandler/proc.cc          |  8 ++++----
 winsup/cygwin/fhandler/process.cc       | 20 ++++++++++----------
 winsup/cygwin/fhandler/procnet.cc       |  8 ++++----
 winsup/cygwin/fhandler/procsysvipc.cc   |  8 ++++----
 winsup/cygwin/fhandler/virtual.cc       |  2 +-
 winsup/cygwin/local_includes/fhandler.h |  4 +++-
 winsup/cygwin/local_includes/path.h     |  8 ++++++++
 winsup/cygwin/path.cc                   |  2 ++
 8 files changed, 36 insertions(+), 24 deletions(-)

diff --git a/winsup/cygwin/fhandler/proc.cc b/winsup/cygwin/fhandler/proc.cc
index a508d4b14d94..7629b54d29db 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 1e5e83b4ab43..20169d22fe25 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 d512887d5f8f..112aee8a2dad 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 453d3b4f7193..b322c4601643 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 4a0d29489e13..90653fbf9a2e 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 bd684a6ea7f3..978d3e5143c8 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 cb8c4ca9f4d6..3dd21d975abf 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 ddea4b3889eb..fa5ceea1065a 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)
 			{

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2024-04-02 13:11 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-02 13:11 [newlib-cygwin/main] Cygwin: fhandler_virtual: move fileid to path_conv member Corinna Vinschen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).