Getopt improvements

[Jeff Mahoney <jeffm@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 2452 bytes --]


	Hey guys -

	Here's a patch against the new reiserfs_getopt stuff.

	There are a few shortcomings with the existing implementation:
	1) There's no easy way to specify the negation of an option. For 
example, in the current implementation, you can do -oremount,attrs - but 
you can't turn them off again. The only way of handling this would have 
been to add another bit.. and then you can't deal with 
-oremount,attrs,noattrs,attrs - which is the correct one in that case? 
Clearly it's the last one, but there's no way to divine that using the 
existing code.

	2) All of the multiple option sets are mutually exclusive. For example, 
-ohash=r5,hash=rupasov would set BOTH FORCE_RUPASOV_HASH and FORCE_R5_HASH.

	The patch addresses both of these problems.
	The first shortcoming is solved by adding a "no_prefix" member to 
opt_desc_t, which allows the option to be negated by prefixing "no" to 
the option text. In addition, I pass in a populated mount_options so 
that bits can be set and cleared on the current running options. The 
result is that as the options are parsed, they are set or cleared in 
order, so -oattrs,noattrs,attrs results in REISERFS_ATTRS being set, but 
-oattrs,noattrs,attrs,noattrs results in it being cleared. This may seem 
silly, but -oremount prepends the mount options from fstab to the mount 
strings, so a filesytem mounted initially with -oattrs, when remounted 
with -oremount,noattrs will actually parse -oremount,attrs,noattrs. I've 
removed the SET_OPT mechanism, since it only sets bits, it doesn't clear 
them. Also, since some options aren't valid during remount (like hashes, 
conversion, etc), I've added a "safe_mask" that only allows the bits 
contained in it to be updated in s_mount_opts.

	The second shortcoming is addressed by clearing all of the bits 
contained in an arg_desc_t array before setting the final value if one 
exists. As with the above solution, the last one on the command line is 
the one that is kept. If, in the future, some multivalue options aren't 
mutually exclusive, it would be trivial to add a "opt_mutex" flag or 
similar to skip this behavior on a per-option basis.

	The end result is this: Except for the addition of "noattrs" as a 
demonstration, the behavior of existing options is unchanged. 
Multi-value options are now handled correctly such that only one of the 
bits can be set at any given time.

	Please apply.

	-Jeff

-- 
jeffm@suse.com
jeffm@csh.rit.edu

[-- Attachment #2: vanilla.getopt.diff --]
[-- Type: text/plain, Size: 5060 bytes --]

Binary files linux-2.4.20.vanilla/fs/reiserfs/.super.c.swp and linux-2.4.20.vanilla.getopt/fs/reiserfs/.super.c.swp differ
diff -ruN linux-2.4.20.vanilla/fs/reiserfs/super.c linux-2.4.20.vanilla.getopt/fs/reiserfs/super.c
--- linux-2.4.20.vanilla/fs/reiserfs/super.c	2002-11-28 18:53:15.000000000 -0500
+++ linux-2.4.20.vanilla.getopt/fs/reiserfs/super.c	2003-02-26 15:03:33.000000000 -0500
@@ -415,6 +415,7 @@
     const arg_desc_t * values; /* list of values accepted by an option */
     int bitmask;  /* bit which is to be set in mount_options bitmask when this
 		     option is selected, 0 is not bits are to be set */
+    int no_prefix : 1; /* "no" can be prefixed to clear this option */
 } opt_desc_t;
 
 
@@ -490,7 +491,13 @@
 	    if (bit_flags && opt->bitmask != -1 )
 		set_bit (opt->bitmask, bit_flags);
 	    break;
-	}
+	} else if (opt->no_prefix && !strncmp (p, "no", 2) &&
+                !strncmp (p + 2, opt->option_name, strlen (opt->option_name))) {
+            if (bit_flags && opt->bitmask != -1)
+                clear_bit (opt->bitmask, bit_flags);
+            p += 2; /* Skip the "no" */
+            break;
+        }
     }
     if (!opt->option_name) {
 	printk ("reiserfs_getopt: unknown option \"%s\"\n", p);
@@ -536,6 +543,14 @@
     /* values possible for this option are listed in opt->values */
     for (arg = opt->values; arg->value; arg ++) {
 	if (!strcmp (p, arg->value)) {
+            const arg_desc_t * arg_clear;
+            /* Clear all the values from this descriptor */
+            for (arg_clear = opt->values; arg_clear->value; arg_clear++) {
+                if (arg_clear->bitmask != -1)
+                    clear_bit (arg_clear->bitmask, bit_flags);
+            }
+
+            /* The the option we found has a bit.. set that */
 	    if (bit_flags && arg->bitmask != -1 )
 		set_bit (arg->bitmask, bit_flags);
 	    return opt->arg_required;
@@ -548,6 +563,7 @@
 
 
 /* returns 0 if something is wrong in option string, 1 - otherwise */
+/* Caller is responsible for clearing mount_options before use if desired */
 static int reiserfs_parse_options (struct super_block * s, char * options, /* string given via mount's -o */
 				   unsigned long * mount_options,
 				   /* after the parsing phase, contains the
@@ -559,18 +575,18 @@
     char * arg = NULL;
     char * pos;
     opt_desc_t opts[] = {
-		{"tails", 't', tails, -1},
-		{"notail", 0, 0, -1}, /* Compatibility stuff, so that -o notail for old setups still work */
-		{"conv", 0, 0, REISERFS_CONVERT}, 
-		{"nolog", 0, 0, -1},
-		{"replayonly", 0, 0, REPLAYONLY},
+		{"tails", 't', tails, -1, 0},
+		{"notail", 0, 0, -1, 0}, /* Compatibility stuff, so that -o notail for old setups still work */
+		{"conv", 0, 0, REISERFS_CONVERT, 0}, 
+		{"nolog", 0, 0, -1, 0},
+		{"replayonly", 0, 0, REPLAYONLY, 0},
 		
-		{"block-allocator", 'a', balloc, -1}, 
-		{"hash", 'h', hash, FORCE_HASH_DETECT},
+		{"block-allocator", 'a', balloc, -1, 0}, 
+		{"hash", 'h', hash, FORCE_HASH_DETECT, 0},
 		
-		{"resize", 'r', 0, -1},
-		{"attrs", 0, 0, REISERFS_ATTRS},
-		{NULL, 0, 0, 0}
+		{"resize", 'r', 0, -1, 0},
+		{"attrs", 0, 0, REISERFS_ATTRS, 1},
+		{NULL, 0, 0, 0, 0}
     };
 	
     *blocks = 0;
@@ -578,9 +594,6 @@
 	/* use default configuration: create tails, journaling on, no
 	   conversion to newest format */
 	return 1;
-    else
-	/* Drop defaults to zeroes */
-	*mount_options = 0;
     
     for (pos = options; pos; ) {
 	c = reiserfs_getopt (s, &pos, opts, &arg, mount_options);
@@ -634,26 +647,26 @@
   struct reiserfs_super_block * rs;
   struct reiserfs_transaction_handle th ;
   unsigned long blocks;
-  unsigned long mount_options = 0;
+  unsigned long mount_options = s->u.reiserfs_sb.s_mount_opt;
+  unsigned long safe_mask = 0;
 
   rs = SB_DISK_SUPER_BLOCK (s);
 
   if (!reiserfs_parse_options(s, data, &mount_options, &blocks))
   	return 0;
 
-#define SET_OPT( opt, bits, super )					\
-    if( ( bits ) & ( 1 << ( opt ) ) )					\
-	    ( super ) -> u.reiserfs_sb.s_mount_opt |= ( 1 << ( opt ) )
-
-  /* set options in the super-block bitmask */
-  SET_OPT( REISERFS_SMALLTAIL, mount_options, s );
-  SET_OPT( REISERFS_LARGETAIL, mount_options, s );
-  SET_OPT( REISERFS_NO_BORDER, mount_options, s );
-  SET_OPT( REISERFS_NO_UNHASHED_RELOCATION, mount_options, s );
-  SET_OPT( REISERFS_HASHED_RELOCATION, mount_options, s );
-  SET_OPT( REISERFS_TEST4, mount_options, s );
-  SET_OPT( REISERFS_ATTRS, mount_options, s );
-#undef SET_OPT
+  /* Add options that are safe here */
+  safe_mask |= 1 << REISERFS_SMALLTAIL;
+  safe_mask |= 1 << REISERFS_LARGETAIL;
+  safe_mask |= 1 << REISERFS_NO_BORDER;
+  safe_mask |= 1 << REISERFS_NO_UNHASHED_RELOCATION;
+  safe_mask |= 1 << REISERFS_HASHED_RELOCATION;
+  safe_mask |= 1 << REISERFS_TEST4;
+  safe_mask |= 1 << REISERFS_ATTRS;
+
+  /* Update the bitmask, taking care to keep
+   * the bits we're not allowed to change here */
+  s->u.reiserfs_sb.s_mount_opt = (s->u.reiserfs_sb.s_mount_opt & ~safe_mask) | (mount_options & safe_mask);
 
   handle_attrs( s );