From 0f30c95a03ecf2a964db566fd8c3e96c1bcb6211 Mon Sep 17 00:00:00 2001 From: Simon Wilkinson Date: Sun, 9 Oct 2011 01:20:57 +0100 Subject: [PATCH] cmd: Add support for pulling options from files With this change, we gain the ability to set our command line options from krb5.conf configuration files. This is only available for tools which are implemented using the new cmd_OptionAs accessor methods. Callers should load their configuration file using cmd_OpenConfigFile("/path/to/config/file.conf"); (an addition to libauth to return a path to a system wide configuration file will be forthcoming) and then set their command name (for example, "fileserver", "afsd", "vos" and so on) using cmd_SetCommandName("mycommand"); The accessor functions will then populate their return values with either: a) The command line options, if specified b) The contents of the tag matching the option name, in the [command_subcommand] section of the configuration file, if it exists c) The contents of the same tag, in the [command] section of the configuration file, if it that exists. d) The contents of the same tag in the [defaults] section of the configuration file. Callers can also gain access to the entire configuration file by calling cmd_RawFile, or to just the section corresponding to their command line by calling cmd_RawSection. Note that when using the file directly, it is up to callers to preserve consistency by implementing similar inheritance rules as the above. Change-Id: Ic501ab296af3638f961486869af79c9ce47b77b8 Reviewed-on: http://gerrit.openafs.org/7135 Reviewed-by: Derrick Brashear Tested-by: BuildBot --- src/butc/Makefile.in | 2 +- src/cmd/Makefile.in | 5 +- src/cmd/cmd.c | 233 +++++++++++++++++++++++++++++++------ src/cmd/cmd.p.h | 5 + src/venus/test/Makefile.in | 3 +- tests/cmd/command-t.c | 33 +++++- tests/cmd/test1.conf | 6 + 7 files changed, 249 insertions(+), 38 deletions(-) create mode 100644 tests/cmd/test1.conf diff --git a/src/butc/Makefile.in b/src/butc/Makefile.in index 8de4bbf820..1dbe5bf591 100644 --- a/src/butc/Makefile.in +++ b/src/butc/Makefile.in @@ -84,7 +84,7 @@ read_tape: read_tape.c -o read_tape ${srcdir}/read_tape.c \ ${TOP_LIBDIR}/libcmd.a ${TOP_LIBDIR}/libusd.a \ ${TOP_LIBDIR}/util.a $(TOP_LIBDIR)/libopr.a \ - $(LIB_roken) + $(LIB_roken) $(XLIBS) # Errors CFLAGS_tcudbprocs.o=@CFLAGS_NOERROR@ diff --git a/src/cmd/Makefile.in b/src/cmd/Makefile.in index ef44d5b7c0..c084b8bd85 100644 --- a/src/cmd/Makefile.in +++ b/src/cmd/Makefile.in @@ -11,7 +11,7 @@ include @TOP_OBJDIR@/src/config/Makefile.lwp LIBOBJS=cmd_errors.o cmd.o config_file.o -LIBPICOBJS=cmd_errors_pic.o cmd_pic.o +LIBPICOBJS=cmd_errors_pic.o cmd_pic.o config_file_pic.o LIB64OBJS=cmd_errors64.o cmd64.o all: ${TOP_LIBDIR}/libcmd.a ${TOP_LIBDIR}/libcmd_pic.a ${TOP_INCDIR}/afs/cmd.h @@ -59,6 +59,9 @@ cmd_errors_pic.o: cmd_errors.c cmd_pic.o: cmd.c cmd.h $(SHD_CCRULE) ${srcdir}/cmd.c +config_file_pic.o: $(TOP_SRCDIR)/external/heimdal/krb5/config_file.c krb5_locl.h + $(SHD_CCRULE) $(TOP_SRCDIR)/external/heimdal/krb5/config_file.c + libcmd64.a: ${LIB64OBJS} AFS_component_version_number64.o -$(RM) -f $@ $(AR) crv $@ ${LIB64OBJS} AFS_component_version_number64.o diff --git a/src/cmd/cmd.c b/src/cmd/cmd.c index 4389adc8bc..89de6ca9d4 100644 --- a/src/cmd/cmd.c +++ b/src/cmd/cmd.c @@ -32,6 +32,8 @@ static int enablePositional = 1; static int enableAbbreviation = 1; static void *beforeRock, *afterRock; static char initcmd_opcode[] = "initcmd"; /*Name of initcmd opcode */ +static cmd_config_section *globalConfig = NULL; +static const char *commandName = NULL; /* take name and string, and return null string if name is empty, otherwise return the concatenation of the two strings */ @@ -1247,18 +1249,95 @@ cmd_ParseLine(char *aline, char **argv, afs_int32 * an, afs_int32 amaxn) } } +/* Read a string in from our configuration file. This checks in + * multiple places within this file - firstly in the section + * [command_subcommand], then in [command], then in [subcommand] + * + * Returns CMD_MISSING if there is no configuration file configured, + * or if the file doesn't contain information for the specified option + * in any of these sections. + */ + +static int +_get_file_string(struct cmd_syndesc *syn, int pos, const char **str) +{ + char *section; + char *optionName; + + /* Nothing on the command line, try the config file if we have one */ + if (globalConfig == NULL) + return CMD_MISSING; + + /* March past any leading -'s */ + for (optionName = syn->parms[pos].name; + *optionName == '-'; optionName++); + + /* First, try the command_subcommand form */ + if (syn->name != NULL && commandName != NULL) { + asprintf(§ion, "%s_%s", commandName, syn->name); + *str = cmd_RawConfigGetString(globalConfig, NULL, section, + optionName, NULL); + free(section); + if (*str) + return 0; + } + + /* Then, try the command form */ + if (commandName != NULL) { + *str = cmd_RawConfigGetString(globalConfig, NULL, commandName, + optionName, NULL); + if (*str) + return 0; + } + + /* Then, the defaults section */ + *str = cmd_RawConfigGetString(globalConfig, NULL, "defaults", + optionName, NULL); + if (*str) + return 0; + + /* Nothing there, return MISSING */ + return CMD_MISSING; +} + +static int +_get_config_string(struct cmd_syndesc *syn, int pos, const char **str) +{ + *str = NULL; + + if (pos > syn->nParms) + return CMD_EXCESSPARMS; + + /* It's a flag, they probably shouldn't be using this interface to access + * it, but don't blow up for now */ + if (syn->parms[pos].items == &dummy) + return 0; + + /* We have a value on the command line - this overrides anything in the + * configuration file */ + if (syn->parms[pos].items != NULL && + syn->parms[pos].items->data != NULL) { + *str = syn->parms[pos].items->data; + return 0; + } + + return _get_file_string(syn, pos, str); +} + int cmd_OptionAsInt(struct cmd_syndesc *syn, int pos, int *value) { - if (pos > syn->nParms) - return CMD_EXCESSPARMS; - if (syn->parms[pos].items == NULL || - syn->parms[pos].items->data == NULL) - return CMD_MISSING; - if (syn->parms[pos].items == &dummy) - return 0; + const char *str; + int code; - *value = strtol(syn->parms[pos].items->data, NULL, 10); + code =_get_config_string(syn, pos, &str); + if (code) + return code; + + if (str == NULL) + return CMD_MISSING; + + *value = strtol(str, NULL, 10); return 0; } @@ -1267,15 +1346,17 @@ int cmd_OptionAsUint(struct cmd_syndesc *syn, int pos, unsigned int *value) { - if (pos > syn->nParms) - return CMD_EXCESSPARMS; - if (syn->parms[pos].items == NULL || - syn->parms[pos].items->data == NULL) - return CMD_MISSING; - if (syn->parms[pos].items == &dummy) - return 0; + const char *str; + int code; - *value = strtoul(syn->parms[pos].items->data, NULL, 10); + code = _get_config_string(syn, pos, &str); + if (code) + return code; + + if (str == NULL) + return CMD_MISSING; + + *value = strtoul(str, NULL, 10); return 0; } @@ -1283,17 +1364,20 @@ cmd_OptionAsUint(struct cmd_syndesc *syn, int pos, int cmd_OptionAsString(struct cmd_syndesc *syn, int pos, char **value) { - if (pos > syn->nParms) - return CMD_EXCESSPARMS; - if (syn->parms[pos].items == NULL || syn->parms[pos].items->data == NULL) - return CMD_MISSING; - if (syn->parms[pos].items == &dummy) - return 0; + const char *str; + int code; if (*value) free(*value); - *value = strdup(syn->parms[pos].items->data); + code = _get_config_string(syn, pos, &str); + if (code) + return code; + + if (str == NULL) + return CMD_MISSING; + + *value = strdup(str); return 0; } @@ -1301,32 +1385,115 @@ cmd_OptionAsString(struct cmd_syndesc *syn, int pos, char **value) int cmd_OptionAsList(struct cmd_syndesc *syn, int pos, struct cmd_item **value) { + const char *str; + struct cmd_item *item, **last; + const char *start, *end; + int code; + if (pos > syn->nParms) return CMD_EXCESSPARMS; - if (syn->parms[pos].items == NULL) - return CMD_MISSING; + + /* If we have a list already, just return the existing list */ + if (syn->parms[pos].items != NULL) { + *value = syn->parms[pos].items; + return 0; + } + + code = _get_file_string(syn, pos, &str); + if (code) + return code; + + /* Use strchr to split str into elements, and build a recursive list + * from them. Hang this list off the configuration structure, so that + * it is returned by any future calls to this function, and is freed + * along with everything else when the syntax description is freed + */ + last = &syn->parms[pos].items; + start = str; + while ((end = strchr(start, ' '))) { + item = calloc(1, sizeof(struct cmd_item)); + item->data = malloc(end - start + 1); + strlcpy(item->data, start, end - start + 1); + *last = item; + last = &item->next; + for (start = end; *start == ' '; start++); /* skip any whitespace */ + } + + /* Catch the final element */ + if (*start != '\0') { + item = calloc(1, sizeof(struct cmd_item)); + item->data = malloc(strlen(start) + 1); + strlcpy(item->data, start, strlen(start) + 1); + *last = item; + } *value = syn->parms[pos].items; + return 0; } int cmd_OptionAsFlag(struct cmd_syndesc *syn, int pos, int *value) { - if (pos > syn->nParms) - return CMD_EXCESSPARMS; - if (syn->parms[pos].items == NULL) - return CMD_MISSING; + const char *str = NULL; + int code; + + code = _get_config_string(syn, pos, &str); + if (code) + return code; + + if (str == NULL || + strcasecmp(str, "yes") == 0 || + strcasecmp(str, "true") == 0 || + atoi(str)) + *value = 1; + else + *value = 0; - *value = 1; return 0; } int cmd_OptionPresent(struct cmd_syndesc *syn, int pos) { - if (pos > syn->nParms || syn->parms[pos].items == NULL) - return 0; + const char *str; + int code; - return 1; + code = _get_config_string(syn, pos, &str); + if (code == 0) + return 1; + + return 0; +} + +int +cmd_OpenConfigFile(const char *file) +{ + if (globalConfig) { + cmd_RawConfigFileFree(globalConfig); + globalConfig = NULL; + } + + return cmd_RawConfigParseFile(file, &globalConfig); +} + +void +cmd_SetCommandName(const char *command) +{ + commandName = command; +} + +const cmd_config_section * +cmd_RawFile(void) +{ + return globalConfig; +} + +const cmd_config_section * +cmd_RawSection(void) +{ + if (globalConfig == NULL || commandName == NULL) + return NULL; + + return cmd_RawConfigGetList(globalConfig, commandName, NULL); } diff --git a/src/cmd/cmd.p.h b/src/cmd/cmd.p.h index 69b5feaa3f..dd0ff4c02e 100644 --- a/src/cmd/cmd.p.h +++ b/src/cmd/cmd.p.h @@ -124,4 +124,9 @@ extern int cmd_RawConfigGetInt(const cmd_config_section *, int, ...); extern const cmd_config_binding *cmd_RawConfigGetList (const cmd_config_section *, ...); +extern int cmd_OpenConfigFile(const char *file); +extern void cmd_SetCommandName(const char *command); +extern const cmd_config_section *cmd_RawFile(void); +extern const cmd_config_section *cmd_RawSection(void); + #endif /* __CMD_INCL__ */ diff --git a/src/venus/test/Makefile.in b/src/venus/test/Makefile.in index f647fd846f..344a06b5e4 100644 --- a/src/venus/test/Makefile.in +++ b/src/venus/test/Makefile.in @@ -14,7 +14,8 @@ include @TOP_OBJDIR@/src/config/Makefile.lwp INCDIRS= -I${TOP_OBJDIR}/src/config -I${TOP_INCDIR} -I.. LDIRS= -L${TOP_LIBDIR} -L.. LIBS= -lsys -lubik -lvldb -lauth -lrxkad -lafshcrypto_lwp \ - -lafscom_err -lcmd -lrx -llwp -lafsutil $(LIB_roken) + -lafscom_err -lcmd -lrx -llwp -lafsutil -lopr \ + $(LIB_roken) $(XLIBS) all test: fulltest owntest idtest getinitparams diff --git a/tests/cmd/command-t.c b/tests/cmd/command-t.c index b08cf5cb4b..e5f4ef67ea 100644 --- a/tests/cmd/command-t.c +++ b/tests/cmd/command-t.c @@ -92,9 +92,10 @@ main(int argc, char **argv) int code; int tc; int retval; + char *path; char *retstring = NULL; - plan(96); + plan(109); initialize_CMD_error_table(); @@ -226,7 +227,8 @@ main(int argc, char **argv) " ... 1st option matches"); is_string("two", retopts->parms[copt_second].items->data, " ... 2nd option matches"); - ok(retopts->parms[copt_flag].items != NULL, " ... 3rd option matches"); + ok(retopts->parms[copt_flag].items != NULL, + " ... 3rd option matches"); cmd_FreeOptions(&retopts); cmd_FreeArgv(tv); @@ -357,6 +359,33 @@ main(int argc, char **argv) cmd_FreeOptions(&retopts); cmd_FreeArgv(tv); + /* Now, try adding a configuration file into the mix */ + if (getenv("SOURCE") == NULL) + path = strdup("test1.conf"); + else + asprintf(&path, "%s/command/test1.conf", getenv("SOURCE")); + + cmd_SetCommandName("test"); + code = cmd_OpenConfigFile(path); + is_int(0, code, "cmd_OpenConfigFile succeeds"); + + code = cmd_ParseLine("-first 1", tv, &tc, 100); + is_int(0, code, "cmd_ParseLine succeeds"); + code = cmd_Parse(tc, tv, &retopts); + is_int(0, code, "cmd_Parse succeeds when we have a config file"); + code = cmd_OptionAsInt(retopts, copt_perhaps, &retval); + is_int(0, code, "cmd_OptionsAsInt succeeds"); + is_int(10, retval, " ... and we have the correct value for perhaps"); + code = cmd_OptionAsString(retopts, copt_sanity, &retstring); + is_int(0, code, "cmd_OptionAsString succeeds"); + is_string("testing", retstring, + " ... and we have the correct value for sanity"); + + /* Check breaking up a list of options */ + code = cmd_OptionAsList(retopts, copt_second, &list); + is_int(0, code, "cmd_OptionAsList succeeds"); + checkList(list, "one", "two", "three", "four", NULL); + return 0; } diff --git a/tests/cmd/test1.conf b/tests/cmd/test1.conf new file mode 100644 index 0000000000..d9c376a1bf --- /dev/null +++ b/tests/cmd/test1.conf @@ -0,0 +1,6 @@ +[test] + perhaps = 10 + second = one two three four + +[defaults] + sanity = testing