From mboxrd@z Thu Jan 1 00:00:00 1970 From: Oleg Drokin Subject: Re: Getopt improvements Date: Thu, 27 Feb 2003 09:25:51 +0300 Message-ID: <20030227092551.C28643@namesys.com> References: <3E5D26A2.7010804@suse.com> Mime-Version: 1.0 Return-path: list-help: list-unsubscribe: list-post: Errors-To: flx@namesys.com Content-Disposition: inline In-Reply-To: <3E5D26A2.7010804@suse.com> List-Id: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Jeff Mahoney Cc: Reiserfs List Hello! On Wed, Feb 26, 2003 at 03:42:10PM -0500, Jeff Mahoney wrote: > 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. All this is highly true. And I got highly annoyed by this myself That's why I developed similar stuff (my patch is against 2.5, see below). Should work with remounting (but necessary two lines are missed in this patch). Also I was in need to set or clear two options, so my approach is a bit different. (Though initially I was going pretty similar stuff to yours). > 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. My version makes it so that only last one takes effect. In fact I just made it possible to specify for each option, what other bits should be cleared when this one is in effect. > 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 Well, I just decided I won't die if I add negated value to description table ;) Bye, Oleg ===== fs/reiserfs/super.c 1.58 vs edited ===== --- 1.58/fs/reiserfs/super.c Tue Jan 14 04:12:01 2003 +++ edited/fs/reiserfs/super.c Thu Feb 27 09:22:34 2003 @@ -500,8 +500,11 @@ mount options that have values rather than being toggles. */ typedef struct { char * value; - int bitmask; /* bit which is to be set in mount_options bitmask when this - value is found, 0 is no bits are to be set */ + int setmask; /* bitmask which is to set on mount_options bitmask when this + value is found, 0 is no bits are to be changed. */ + int clrmask; /* bitmask which is to clear on mount_options bitmask when this + value is found, 0 is no bits are to be changed. This is + applied BEFORE setmask */ } arg_desc_t; @@ -511,25 +514,30 @@ char * option_name; int arg_required; /* 0 if argument is not required, not 0 otherwise */ 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 setmask; /* bitmask which is to set on mount_options bitmask when this + value is found, 0 is no bits are to be changed. */ + int clrmask; /* bitmask which is to clear on mount_options bitmask when this + value is found, 0 is no bits are to be changed. This is + applied BEFORE setmask */ } opt_desc_t; /* possible values for "-o block-allocator=" and bits which are to be set in s_mount_opt of reiserfs specific part of in-core super block */ static const arg_desc_t balloc[] = { - {"noborder", REISERFS_NO_BORDER}, - {"no_unhashed_relocation", REISERFS_NO_UNHASHED_RELOCATION}, - {"hashed_relocation", REISERFS_HASHED_RELOCATION}, - {"test4", REISERFS_TEST4}, - {NULL, -1} + {"noborder", 1<option_name; opt ++) { if (!strncmp (p, opt->option_name, strlen (opt->option_name))) { - if (bit_flags && opt->bitmask != -1) - set_bit (opt->bitmask, bit_flags); + if (bit_flags) { + *bit_flags &= ~opt->clrmask; + *bit_flags |= opt->setmask; + } break; } } @@ -623,8 +633,10 @@ /* values possible for this option are listed in opt->values */ for (arg = opt->values; arg->value; arg ++) { if (!strcmp (p, arg->value)) { - if (bit_flags && arg->bitmask != -1 ) - set_bit (arg->bitmask, bit_flags); + if (bit_flags) { + *bit_flags &= ~arg->clrmask; + *bit_flags |= arg->setmask; + } return opt->arg_required; } } @@ -633,7 +645,6 @@ return -1; } - /* returns 0 if something is wrong in option string, 1 - otherwise */ static int reiserfs_parse_options (struct super_block * s, char * options, /* string given via mount's -o */ unsigned long * mount_options, @@ -647,17 +658,19 @@ 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}, - {"attrs", 0, 0, REISERFS_ATTRS}, - {"nolog", 0, 0, -1}, - {"replayonly", 0, 0, REPLAYONLY}, - {"block-allocator", 'a', balloc, -1}, - {"resize", 'r', 0, -1}, - {"jdev", 'j', 0, -1}, - {NULL, 0, 0, -1} + {"tails", 't', tails, 0, 0}, + /* Compatibility stuff, so that -o notail + for old setups still work */ + {"notail", 0, 0, 0, (1<