code style in the vfs

[Roland Illig <roland illig gmx de>]

Hi,

I have rewritten a small part of the vfs. I noticed that the function 
_vfs_get_class -- although declared to take a "const char *" cannot be 
fed with a string literal, because it modifies the path. This has led to 
two changes:

1. I extended my private src/global.h (which I will not commit unless 
asked to do it):

#ifdef strchr
#undef strchr
#endif
#define strchr(m_str, m_chr) (const char *) (strchr) (m_str, m_chr)
#define mod_strchr(m_str, m_chr) (strchr) (m_str, m_chr)

The point is that strchr takes a "const char *", but it returns a "char 
*", which can point to the "const char *", effectively discarding the 
"const". Using these definitions I get some more warnings which I will 
fix in the next time. There are many more functions which are declared 
inappropriately, like strchr, and I will try to find them all.

If you agree that there are actually two strchr functions, which could 
be overloaded in C++, perhaps we can discuss if we want to call them 
with distinct names, like I have done in my #defines above. The two 
strchr functions are:

const char *strchr(const char *, int);
char *strchr(char *, int);

2. Many vfs functions make a local copy of a string, change only one 
character and then pass that string to other functions. For example:

    slash = strchr (semi, PATH_SEP);
    if (slash)
        *slash = 0;
    ret = vfs_prefix_to_class (semi + 1);

This pattern requires the massive use of g_strdup, which can be 
annoying, because the string duplicates must be freed later.

My approach has been to change string parameters into two parameters, 
the string and the string_len. This eliminates the need for most of the 
cases like the example above.

So how do you think about it? Is it worth modifying some (mostly 
internal) functions to get to the (string,length) representation of strings?

Roland

Index: vfs/vfs.c
===================================================================
RCS file: /cvsroot/mc/mc/vfs/vfs.c,v
retrieving revision 1.168
diff -u -r1.168 vfs.c
--- vfs/vfs.c	24 Sep 2004 16:03:24 -0000	1.168
+++ vfs/vfs.c	24 Sep 2004 19:48:49 -0000
@@ -150,7 +150,7 @@
 
 /* Return VFS class for the given prefix */
 static struct vfs_class *
-vfs_prefix_to_class (char *prefix)
+vfs_prefix_to_class (const char *prefix, size_t prefix_len)
 {
     struct vfs_class *vfs;
 
@@ -160,8 +160,7 @@
 		continue;
 	    return vfs;
 	}
-	if (vfs->prefix
-	    && !strncmp (prefix, vfs->prefix, strlen (vfs->prefix)))
+	if (vfs->prefix && strncmp (prefix, vfs->prefix, prefix_len) == 0)
 	    return vfs;
     }
     return NULL;
@@ -243,7 +242,7 @@
     if (slash)
 	*slash = 0;
 
-    if ((ret = vfs_prefix_to_class (semi+1))){
+    if ((ret = vfs_prefix_to_class (semi+1, strlen(semi+1)))){
 	if (op) 
 	    *op = semi + 1;
 	if (inpath)
@@ -260,10 +259,10 @@
 }
 
 static struct vfs_class *
-_vfs_get_class (const char *path)
+_vfs_get_class (const char *path, size_t path_len)
 {
-    char *semi;
-    char *slash;
+    const char *semi;
+    const char *slash;
     struct vfs_class *ret;
 
     g_return_val_if_fail(path, NULL);
@@ -273,18 +272,14 @@
 	return NULL;
     
     slash = strchr (semi, PATH_SEP);
-    *semi = 0;
-    if (slash)
-	*slash = 0;
-    
-    ret = vfs_prefix_to_class (semi+1);
+    if (slash == NULL)
+        slash = semi + strlen(semi);
+
+    ret = vfs_prefix_to_class (semi+1, slash - (semi+1));
 
-    if (slash)
-	*slash = PATH_SEP;
     if (!ret)
-	ret = _vfs_get_class (path);
+	ret = _vfs_get_class (path, semi - path);
 
-    *semi = '#';
     return ret;
 }
 
@@ -293,7 +288,7 @@
 {
     struct vfs_class *vfs;
 
-    vfs = _vfs_get_class(path);
+    vfs = _vfs_get_class(path, strlen(path));
 
     if (!vfs)
 	vfs = localfs_class;
@@ -564,7 +559,7 @@
     char *p;
     struct stat my_stat, my_stat2;
 
-    if (!_vfs_get_class (current_dir)) {
+    if (!_vfs_get_class (current_dir, strlen(current_dir))) {
 	p = g_get_current_dir ();
 	if (!p)			/* One of the directories in the path is not readable */
 	    return current_dir;