mysql-proxy-discuss team mailing list archive

[nick loeve <trickie@xxxxxxxxx>]

Hey!

I get so lost in the option parsing code also. The new funnel version
im slowly working on has no config options at the moment, so I am
flexible :)

I think GOptionGroup's are good, as you get the per-group help for
free, and like you say having the user_data param gives you extra
flexibility.

My 2cents

On Thu, Jul 30, 2009 at 2:00 PM, Kay Röpke<Kay.Roepke@xxxxxxx> wrote:
> Hi!
>
> (Nick, I cc'ed you on this because it likely directly affects funnel.
> Probably in a trivial way, but nonetheless.)
>
> Looking at bug#46048 <http://bugs.mysql.com/46048> I realized that our
> option handling code is a bit suboptimal.
> While in general it works just fine, and fixing the bug can be done with
> minimal effort, it's a bit cluttered.
> BTW, this bug is simply that we can't figure out if an INT option has been
> set on the command line or via an assignment (most likely assigning the
> default value) in code when parsing the defaults file.
> For strings and string arrays it works, doubles (which we don't use
> currently) would be even worse to handle, since you can't rely on a double
> fitting into a pointer.
>
> To expand on the current implementation a bit:
> Currently we base everything on GOptionEntry, which does not know about
> default values.
> What's more, large chunks of main_cmdline deals with options, making the
> code a bit harder to follow than necessary, especially figuring out at what
> points config variables can change their values.
> The plugins are a bit cleaner in this regard, since they will generally
> initialize their variables in their init function, set the GOptionEntries in
> get_options and do something based on the actual values in apply_config.
>
> Nevertheless, it would be nice to abstract out the entire options vs
> keyfiles vs default values process, but this will likely mean breaking
> binary compatibility with older plugins (which means you wouldn't be able to
> load them into a newer chassis). That's fine, really, since most of the time
> no one does that anyway (at least I don't know of anyone who does).
>
> 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).
>
> 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.
>
> 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).
>
> Thoughts, opinions and critique to me please :)
>
> 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
>
>



-- 
Nick Loeve