← Back to team overview

mysql-proxy-discuss team mailing list archive

Re: Refactoring cmdline option and default handling

 


On Aug 5, 2009, at 5:03 PM, Jan Kneschke wrote:


Am 30.07.2009 um 14:00 schrieb Kay Röpke:

Here's my proposal in generic terms (haven't fully spec'ed it out):
Wrap all chassis config variables in a struct, makes it easy to see which ones are influenced by the outside world. Rather than using GOptionEntry directly, wrap it in a struct as well, alongside a union holding the different possible default values. Construction of an argument "object" would mean a function call with pretty much what GOptionEntry contains, either followed by a function call to set the default if there is any, or via some pointer trickery in the construction function directly (i.e. pass in void* and deref/cast to the correct type based on the G_OPTION_ARG enum).

Instead of passing a default value down, just provide a way to figure out if a value got set. For something like "basedir" we only have to get a good automatic basedir if nothing is provided by the user.

the problem is that there's no way to do this for scalar values. strings are fine, since we can compare them to NULL, but integers and doubles will most likely be initialized to 0 and we can't differentiate between "the user has set it to 0 on the cmdline" and "the default was 0". this differentiation is necessary for the keyfile handling to work correctly (i.e. be overridable from the cmdline).

With that in mind, the wrapping struct would only get a gboolean for "is_set" and in the global "apply_config" we would iterate over the options and do the post-processing.

for the is_set bool we need to compare it to the default value, so we are back to square one, iiuc.

To get the actual array of GOptionEntry structs, you'd call a function once you registered all the options. Optionally you could request a GOptionGroup directly, haven't made up my mind on that one yet. Using option groups directly have the added benefit of allowing to pass in a user_data, which would contain the default values. Ideally I'd like to hide all the complexity that allowing the defaults file brings with a post parse hook, also provided by the option groups. This would be a function in the chassis that deals with figuring out the defaults file implications.

That's the tricky part: the GOption* API doesn't have a way to get back a group from context nor get the options from a group. That's all internal.

The post_parse function gets call after the group is parsed, but there is no real way that I can see how to intercept a "option is set".

we wouldn't need that, it should be enough to compare all the actual values of the arg_data variable to the default passed in via the user_data in the post parse hook.
this implies one crucial aspect of all this:
we need to make sure the actual variables are not initialized to the default value! otherwise we couldn't differentiate the scalars again. so the piece of code that constructs the GOptionEntry structs needs to set the arg_data pointer's target to a suitable marker value which is extremely unlikely to ever happen. that might be tricky, but i'm sure we can find values that are suitable.

 http://library.gnome.org/devel/glib/unstable/glib-Commandline-option-parser.html#GOptionParseFunc

Perhaps we can somehow map everything to G_OPTION_ARG_CALLBACK which will fire a callback for each value which allows us to track the "option is set" and then does the real value setting.

 http://library.gnome.org/devel/glib/unstable/glib-Commandline-option-parser.html#GOptionArgFunc

I think with callbacks you have to do the parsing as well, which I think is not what we want. In fact, all we want is a default.

To see if value is set in the config-file is straight forward:

 http://library.gnome.org/devel/glib/unstable/glib-Key-value-file-parser.html#g-key-file-has-key

sure, but that is not our problem, really. when it comes to the point where we read the keyfile the options have already been parsed and set.

which gives me a different idea, actually. maybe it becomes easier if i try to switch the process around. i'll experiment a bit, since my proposal turned out to be quite a bit of new code, too. it's trivial code, but nonetheless yet more code to test and read. less code == good code, no code == best code :)


The neat thing would be that almost all of the complexity of dealing with options would be hidden, making the available options, their storage location and their default values painfully obvious in the code. And it hopefully cuts down on the size of main_cmdline - I always nearly get lost in there, it's too long (which also address part of one of the review comments we received).

I'm all for the overall goal of hiding the complexity. chassis_keyfile.c is pretty empty up to now :)


yup :)

cheers,
-k
--
Kay Roepke
Software Engineer, MySQL Enterprise Tools

Sun Microsystems GmbH    Sonnenallee 1, DE-85551 Kirchheim-Heimstetten
Geschaeftsfuehrer:    Thomas Schroeder,  Wolfang Engels,  Wolf Frenkel
Vorsitz d. Aufs.rat.: Martin Haering                    HRB MUC 161028




References