accre / lstore-lio

Implementation of the L-Store filesystem
http://www.lstore.org
1 stars 1 forks source link

Change command line/config parsing. #11

Open PerilousApricot opened 9 years ago

PerilousApricot commented 9 years ago

I see two things that are slightly coupled that could be done to keep weird arguments/configs from causing the command line tool from segfaulting.

Instead of the (somewhat) ad-hoc way we bring up configurations https://github.com/accre/lstore-lio/blob/master/lio_config.c#L818 and load arguments https://github.com/accre/lstore-lio/blob/master/lio_config.c#L1159 and write the "usage" help message https://github.com/accre/lstore-lio/blob/master/lio_config.c#L699 , I want to unify them in a way that will both help us auto-document all the different parameters, add some type safety, and bake in the defaults into one place.

I see something like the following:

typedef enum { ARG_INT, ARG_UINT, ARG_SECTION, /* other types */ } lio_arg_desc_type_enum;
typedef union { int i, uint ui,  /* other types */ } lio_arg_desc_type_union;
typedef struct {
    const char * name;
    const char * description;
    lio_arg_desc_type_enum type;
    lio_arg_desc_type_union default;
} lio_arg_desc_t;

#define INT_VALUE(n,d,z) { .name = n, .description = d, .type = ARG_INT, .default.i = z }
/* similar defines for other types */

const lio_arg_desc_t inner_desc[] = {
    INT_VALUE("bar", "How many bars should we have", 1)
}
const lio_arg_desc_t top_desc[] = {
    INT_VALUE("foo", "How many foos should we have", 0),
    INT_VALUE("probs", "Number of problems", 99),
    SECTION_VALUE("inner", "Stores information about bars", inner_desc),
}

The same machinery can be used for command line arguments, which will chuck out an inip with the input values instead of a bunch of variables.

In both cases, we can then do an introspection of variables passed in and alert the user early that they're giving us something wrong. I constantly pass the wrong things in and get a segfault, which isn't user friendly. If we know early enough, we can panic sensibly. We also describe fully from a single canonical source the arguments expected/allowed. This will let us also let us spit out things like, "What does a default configuration look like" automagically.

Once those two are using the "same" machinery, we can then enable things like having "-o inner.bar=99" on the command line, which override values in the configuration. Or perhaps letting environment variables set values as well.

tacketar commented 9 years ago

The thing to be cognizant of is multi-arg command line options AND duplicates support. A quick example I use all the time is the blacklist argument "-bl key value" on lio_inspect. It takes 2 arguments and can be repeated multiple times to blacklist multiple rids, ie "-bl rid_key 1234 -bl host cms-depot5.vampire"

Alan

On 10/26/2015 12:37 PM, Andrew Melo wrote:

I see two things that are slightly coupled that could be done to keep weird arguments/configs from causing the command line tool from segfaulting.

  • Move command line parsing to getopt
  • Rejigger ini parsing

Instead of the (somewhat) ad-hoc way we bring up configurations https://github.com/accre/lstore-lio/blob/master/lio_config.c#L818 and load arguments https://github.com/accre/lstore-lio/blob/master/lio_config.c#L1159 and write the "usage" help message https://github.com/accre/lstore-lio/blob/master/lio_config.c#L699 , I want to unify them in a way that will both help us auto-document all the different parameters, add some type safety, and bake in the defaults into one place.

I see something like the following:

typedef enum { ARG_INT, ARG_UINT, ARGSECTION,/* other types / } lio_arg_desc_typeenum; typedef union {int i,uint ui,/ other types / } lio_arg_desc_type_union; typedef struct { const char * name; const char \ description; lio_arg_desc_type_enum type; lio_arg_desc_type_uniondefault; }lio_arg_desc_t;

define INT_VALUE(n,d,z) { .name = n, .description = d, .type = ARG_INT, .default.i = z }

/* similar defines for other types */

const lio_arg_desc_t inner_desc[] = { INT_VALUE("bar","How many bars should we have",1) } const lio_arg_desc_t top_desc[] = { INT_VALUE("foo","How many foos should we have",0), INT_VALUE("probs","Number of problems",99), SECTION_VALUE("inner","Stores information about bars", inner_desc), }

The same machinery can be used for command line arguments, which will chuck out an inip with the input values instead of a bunch of variables.

In both cases, we can then do an introspection of variables passed in and alert the user early that they're giving us something wrong. I constantly pass the wrong things in and get a segfault, which isn't user friendly. If we know early enough, we can panic sensibly. We also describe fully from a single canonical source the arguments expected/allowed. This will let us also let us spit out things like, "What does a default configuration look like" automagically.

Once those two are using the "same" machinery, we can then enable things like having "-o inner.bar=99" on the command line, which override values in the configuration. Or perhaps letting environment variables set values as well.

— Reply to this email directly or view it on GitHub https://github.com/accre/lstore-lio/issues/11.

PerilousApricot commented 9 years ago

Yeah, I forsee there being some sort of 'options' bitvector field where you could do ARG_DUPLICATES | ARG_REQUIRED or similar.

PerilousApricot commented 9 years ago

I'll wanna think about it a bit more. Perhaps it makes sense to give the inip stuff the ability to natively store arrays, then have something like inip_get_array_integer(key, offset). I need to go through and at categorize all of the different usages first to make sure the functionality supports things right.