Re: Option reorganisation, Part IV.

[Zefram <zefram@xxxxxxxxxxxxxxxxx>]

Zoltan Hidvegi wrote:
>After this patch the optns array is no longer used.  Loops over this array
>are replaced with scanhashtable and scanmatchtable calls.

But it *is* used.  It has to be used to insert the initial options into
the hashtable.  Any use of it after that we get for free, so there's no
reason not to look up the flags and so on there.  And walking through
the table is faster than scanhashtable(), for those functions that need
to look at all the options.

>After this patch it'll be quite easy to add new options dynamically.

But do we *want* to?  I don't like the idea of options that can't be
set until the appropriate module has been loaded.  And autoloaded
options would be silly, and would cause unnecessary module loading.  I
think options should really affect only the core functionality.
Anything else can use command line switches or parameters.

You store aliases in a manner that requires a second name lookup, but
it's easier and more efficient to store the actual option number.  (And
the way you handle the structures with the union is not strictly
conforming, and quite likely to break on 64-bit machines using the LP64
model.)  I don't see the point in doing this.

You also changed the {z,k}shletters arrays to be of the enum type,
rather than short.  This is a bad idea.  On 32-bit machines the enum is
most likely larger -- if we really want the extra speed in table
lookups we should be more sure of it by using int.  More importantly,
the enum could be unsigned, but the table contains negative values.
And there are probably some compilers that would warn about the use of
a value that is not one of the enumeration constants.  (For these
reasons, I *never* use an enum as a type.)

-zefram