On Tue, Feb 26, 2002 at 12:39:47PM -0000, Chris January wrote: >> 1) The copyrights still need to be changed. >Done. >> 2) The code formatting still is not correct. >Now piped through indent with a few touch-ups. >> 3) You have a lot of calls to normalize_posix_path. Is that really >> necessary? It seems to be called a lot. If it is really necessary, >> I'd prefer that it just be called in dtable::build_fhandler and made >> the standard "unix_path_name". >Done. >> 4) Could you generate the diff using 'cvs diff -up" >Done. The new files are diff'ed against /dev/null and are appended to the >output of cvs diff. > >NB: The ChangeLog has two additions because I found chdir had broken. > >@@ -370,9 +379,9 @@ dtable::build_fhandler (int fd, DWORD de > > if (unix_name) > { >- char new_win32_name[strlen (unix_name) + 1]; > if (!win32_name) > { >+ char new_win32_name[strlen (unix_name) + 1]; > char *p; > /* FIXME: ? Should we call win32_device_name here? > It seems like overkill, but... */
The above is a gratuitous and dangerous change. >@@ -100,7 +103,10 @@ enum > extern const char *windows_device_names[]; > extern struct __cygwin_perfile *perfile_table; > #define __fmode (*(user_data->fmode_ptr)) >+extern const char *proc; >+extern const int proc_len; > >+ > class select_record; > class path_conv; > class fhandler_disk_file; Please eliminate the extra white space. >@@ -1028,6 +1034,74 @@ class fhandler_dev_dsp : public fhandler > void fixup_after_exec (HANDLE); > }; > >+class fhandler_virtual : public fhandler_base >+{ >+ protected: >+ char normalized_path[MAX_PATH]; There should be no need for normalized_path, should there? >@@ -490,6 +496,24 @@ path_conv::check (const char *src, unsig > } > goto out; > } >+ else if (isvirtual_dev(devn)) >+ { >+ fhandler_virtual *fh = >+ (fhandler_virtual *)cygheap->fdtab.build_fhandler(-1, devn, >path_copy, NULL, unit); >+ int file_type = fh->exists(path_copy); >+ switch (file_type) >+ { >+ case 0: >+ error = ENOENT; >+ case 1: >+ case 2: >+ fileattr = FILE_ATTRIBUTE_DIRECTORY; >+ case -1: >+ fileattr = 0; >+ } >+ delete fh; >+ return; >+ } > /* devn should not be a device. If it is, then stop parsing now. */ > else if (devn != FH_BAD) > { Non GNU-formatting above. Need spaces before parentheses in function calls. Need spaces after closing parentheses in coercions. Please just fix this. Don't run the code through indent. >@@ -1405,10 +1429,16 @@ mount_info::conv_to_win32_path (const ch > else if (mount_table->cygdrive_len > 1) > return ENOENT; > } >+ if (isproc (pathbuf)) >+ { >+ devn = fhandler_proc::get_proc_fhandler(pathbuf); >+ dst[0] = '\0'; >+ goto out; >+ } > > int chrooted_path_len; > chrooted_path_len = 0; > /* Check the mount table for prefix matches. */ > for (i = 0; i < nmounts; i++) > { > const char *path; Ditto. >@@ -1472,7 +1502,7 @@ mount_info::conv_to_win32_path (const ch > *flags = mi->flags; > } > >- if (devn != FH_CYGDRIVE) >+ if (!isvirtual_dev(devn)) > win32_device_name (src_path, dst, devn, unit); > > out: Ditto. >@@ -3233,7 +3263,8 @@ chdir (const char *in_dir) > path.get_win32 ()[3] = '\0'; > } > int res; >- if (path.get_devn () != FH_CYGDRIVE) >+ int devn = path.get_devn(); >+ if (!isvirtual_dev(devn)) > res = SetCurrentDirectory (native_dir) ? 0 : -1; > else > { Ditto. >@@ -3253,8 +3284,9 @@ chdir (const char *in_dir) > we'll see if Cygwin mailing list users whine about the current behavior. */ > if (res == -1) > __seterrno (); >- else if (!path.has_symlinks () && strpbrk (dir, ":\\") == NULL >- && pcheck_case == PCHECK_RELAXED) >+ else if ((!path.has_symlinks () && strpbrk (dir, ":\\") == NULL >+ && pcheck_case == PCHECK_RELAXED) || >+ isvirtual_dev(devn)) > cygheap->cwd.set (native_dir, dir); > else > cygheap->cwd.set (native_dir, NULL); Ditto. >--- /dev/null Tue Feb 26 12:30:52 2002 >+++ fhandler_proc.cc Tue Feb 26 10:15:16 2002 >@@ -0,0 +1,294 @@ >+/* offsets in proc_listing */ >+static const int PROC_REGISTRY = 0; // /proc/registry >+static const int PROC_VERSION = 1; // /proc/version >+static const int PROC_UPTIME = 2; // /proc/uptime >+static const int PROC_LINK_COUNT = 3; >+ >+/* names of objects in /proc */ >+static const char *proc_listing[PROC_LINK_COUNT] = { >+ "registry", >+ "version", >+ "uptime" >+}; Either derive PROC_LINK_COUNT from the size of the array or just use a NULL terminated array (preferred). >+ >+/* name of the /proc filesystem */ >+const char *proc = "/proc"; >+const int proc_len = strlen (proc); How about: const char proc[] = "/proc"; const int proc_len = sizeof (proc) - 1; Then we don't have any runtime hits. >+/* auxillary function that returns the fhandler associated with the given path >+ * this is where it would be nice to have pattern matching in C - polymorphism >+ * just doesn't cut it >+ */ >+DWORD >+fhandler_proc::get_proc_fhandler (const char *path) >+{ >+ debug_printf("get_proc_fhandler(%s)", path); >+ path += proc_len; >+ while (SLASH_P (*path)) >+ path++; Why are you eating slashes here? Unnecessary slashes should have been eliminated by normalize_path. >+ if (*path == 0) >+ return FH_PROC; How could this happen? >+ for (int i = 0; i < PROC_LINK_COUNT; i++) >+ { >+ if (path_prefix_p (proc_listing[i], path, strlen (proc_listing[i]))) >+ return proc_fhandlers[i]; >+ } Here is where you could just do: for (int i = 0; proc_listing[i]; i++) >+ int pid = atoi (path); >+ winpids pids; >+ for (unsigned i = 0; i < pids.npids; i++) >+ { >+ _pinfo *p = pids[i]; >+ >+ if (!proc_exists (p)) >+ continue; >+ >+ if (p->pid == pid) >+ return FH_PROCESS; >+ } >+ return FH_PROC; >+} Is this right? If I type /proc/qwerty this returns FH_PROC? >--- /dev/null Tue Feb 26 12:30:59 2002 >+++ fhandler_process.cc Tue Feb 26 10:01:16 2002 >@@ -0,0 +1,289 @@ >+/* fhandler_process.cc: fhandler for /proc/<pid> virtual filesystem >+ >+ Copyright 2002 Red Hat, Inc. >+ >+This file is part of Cygwin. >+ >+This software is a copyrighted work licensed under the terms of the >+Cygwin license. Please consult the file "CYGWIN_LICENSE" for >+details. */ >+ >+#include "winsup.h" >+#include <sys/fcntl.h> >+#include <errno.h> >+#include <unistd.h> >+#include <stdlib.h> >+#include <sys/cygwin.h> >+#include "cygerrno.h" >+#include "security.h" >+#include "fhandler.h" >+#include "sigproc.h" >+#include "pinfo.h" >+#include "path.h" >+#include "shared_info.h" >+#include <assert.h> >+ >+#define _COMPILING_NEWLIB >+#include <dirent.h> >+ >+static const int PROCESS_PPID = 0; >+static const int PROCESS_EXENAME = 1; >+static const int PROCESS_WINPID = 2; >+static const int PROCESS_WINEXENAME = 3; >+static const int PROCESS_STATUS = 4; >+static const int PROCESS_UID = 5; >+static const int PROCESS_GID = 6; >+static const int PROCESS_PGID = 7; >+static const int PROCESS_SID = 8; >+static const int PROCESS_CTTY = 9; >+static const int PROCESS_LINK_COUNT = 10; >+ >+static const char *process_listing[PROCESS_LINK_COUNT] = { >+ "ppid", >+ "exename", >+ "winpid", >+ "winexename", >+ "status", >+ "uid", >+ "gid", >+ "pgid", >+ "sid", >+ "ctty" >+}; Same comments as before wrt PROCESS_LINK_COUNT. >--- /dev/null Tue Feb 26 12:31:02 2002 >+++ fhandler_registry.cc Tue Feb 26 11:11:53 2002 >@@ -0,0 +1,505 @@ >+const int registry_len = strlen ("registry"); >+/* If this bit is set in __d_position then we are enumerating values, >+ * else sub-keys. keeping track of where we are is horribly messy >+ * the bottom 16 bits are the absolute position and the top 15 bits >+ * make up the value index if we are enuerating values. >+ */ >+const __off32_t REG_ENUM_VALUES_MASK = 0x8000000; >+ >+const int ROOT_KEY_COUNT = 7; Derive from array. Should these be static? >+/* List of root keys in /proc/registry. >+ * Possibly we should filter out those not relevant to the flavour of Windows >+ * Cygwin is running on. >+ */ >+static const char *registry_listing[ROOT_KEY_COUNT] = { >+ "HKEY_CLASSES_ROOT", >+ "HKEY_CURRENT_CONFIG", >+ "HKEY_CURRENT_USER", >+ "HKEY_LOCAL_MACHINE", >+ "HKEY_USERS", >+ "HKEY_DYN_DATA", // 95/98/Me >+ "HKEY_PERFOMANCE_DATA" // NT/2000/XP >+}; >+static HKEY registry_keys[ROOT_KEY_COUNT] = { HKEY_CLASSES_ROOT, >+ HKEY_CURRENT_CONFIG, >+ HKEY_CURRENT_USER, >+ HKEY_LOCAL_MACHINE, >+ HKEY_USERS, >+ HKEY_DYN_DATA, >+ HKEY_PERFORMANCE_DATA >+}; >--- /dev/null Tue Feb 26 12:31:06 2002 >+++ fhandler_virtual.cc Tue Feb 26 11:14:04 2002 >@@ -0,0 +1,228 @@ >+DIR * >+fhandler_virtual::opendir (path_conv & real_name) >+{ >+ return opendir (get_name()); >+} I don't see how this can be right. Won't this induce infinite recursion? Phew. cgf