From 60f0f09d589d676eac93180f86f66170e1be4ccc Mon Sep 17 00:00:00 2001 From: "Christopher J. Morrone" <morrone2@llnl.gov> Date: Fri, 21 Jul 2006 23:06:08 +0000 Subject: [PATCH] Make the config file parser more strict. Now, there may only be ONE key-value pair per line, EXCEPT on lines that BEGIN with key-value pairs such as NodeName and PartitionName, which explicitly accept additional values. This allows us to eliminate redundant listing of ignored keys... --- src/common/parse_config.c | 135 ++++++++++++------ src/common/parse_config.h | 32 +++-- src/common/read_config.c | 128 +++++++---------- .../block_allocator/block_allocator.c | 10 +- .../block_allocator/block_allocator.h | 2 +- 5 files changed, 166 insertions(+), 141 deletions(-) diff --git a/src/common/parse_config.c b/src/common/parse_config.c index 3f383878eb4..65c5431d07e 100644 --- a/src/common/parse_config.c +++ b/src/common/parse_config.c @@ -70,7 +70,8 @@ struct s_p_values { int data_count; void *data; int (*handler)(void **data, slurm_parser_enum_t type, - const char *key, const char *value, const char *line); + const char *key, const char *value, + const char *line, char **leftover); void (*destroy)(void *data); s_p_values_t *next; }; @@ -199,7 +200,7 @@ static void _keyvalue_regex_init(void) if (!keyvalue_initialized) { if (regcomp(&keyvalue_re, keyvalue_pattern, REG_EXTENDED) != 0) { - /* FIXME - should be fatal */ + /* FIXME - should be fatal? */ error("keyvalue regex compilation failed\n"); } keyvalue_initialized = true; @@ -362,7 +363,7 @@ static int _get_next_line(char *buf, int buf_size, FILE *file) } static int _handle_string(s_p_values_t *v, - const char *value, const char *line) + const char *value, const char *line, char **leftover) { if (v->data_count != 0) { error("%s specified more than once", v->key); @@ -372,7 +373,8 @@ static int _handle_string(s_p_values_t *v, if (v->handler != NULL) { /* call the handler function */ int rc; - rc = v->handler(&v->data, v->type, v->key, value, line); + rc = v->handler(&v->data, v->type, v->key, value, + line, leftover); if (rc != 1) return rc == 0 ? 0 : -1; } else { @@ -384,7 +386,7 @@ static int _handle_string(s_p_values_t *v, } static int _handle_long(s_p_values_t *v, - const char *value, const char *line) + const char *value, const char *line, char **leftover) { if (v->data_count != 0) { error("%s specified more than once", v->key); @@ -394,7 +396,8 @@ static int _handle_long(s_p_values_t *v, if (v->handler != NULL) { /* call the handler function */ int rc; - rc = v->handler(&v->data, v->type, v->key, value, line); + rc = v->handler(&v->data, v->type, v->key, value, + line, leftover); if (rc != 1) return rc == 0 ? 0 : -1; } else { @@ -424,7 +427,7 @@ static int _handle_long(s_p_values_t *v, } static int _handle_uint16(s_p_values_t *v, - const char *value, const char *line) + const char *value, const char *line, char **leftover) { if (v->data_count != 0) { error("%s specified more than once", v->key); @@ -434,7 +437,8 @@ static int _handle_uint16(s_p_values_t *v, if (v->handler != NULL) { /* call the handler function */ int rc; - rc = v->handler(&v->data, v->type, v->key, value, line); + rc = v->handler(&v->data, v->type, v->key, value, + line, leftover); if (rc != 1) return rc == 0 ? 0 : -1; } else { @@ -472,7 +476,7 @@ static int _handle_uint16(s_p_values_t *v, } static int _handle_uint32(s_p_values_t *v, - const char *value, const char *line) + const char *value, const char *line, char **leftover) { if (v->data_count != 0) { error("%s specified more than once", v->key); @@ -482,7 +486,8 @@ static int _handle_uint32(s_p_values_t *v, if (v->handler != NULL) { /* call the handler function */ int rc; - rc = v->handler(&v->data, v->type, v->key, value, line); + rc = v->handler(&v->data, v->type, v->key, value, + line, leftover); if (rc != 1) return rc == 0 ? 0 : -1; } else { @@ -521,7 +526,7 @@ static int _handle_uint32(s_p_values_t *v, } static int _handle_pointer(s_p_values_t *v, - const char *value, const char *line) + const char *value, const char *line, char **leftover) { if (v->data_count != 0) { error("%s specified more than once", v->key); @@ -531,7 +536,8 @@ static int _handle_pointer(s_p_values_t *v, if (v->handler != NULL) { /* call the handler function */ int rc; - rc = v->handler(&v->data, v->type, v->key, value, line); + rc = v->handler(&v->data, v->type, v->key, value, + line, leftover); if (rc != 1) return rc == 0 ? 0 : -1; } else { @@ -543,7 +549,7 @@ static int _handle_pointer(s_p_values_t *v, } static int _handle_array(s_p_values_t *v, - const char *value, const char *line) + const char *value, const char *line, char **leftover) { void *new_ptr; void **data; @@ -551,7 +557,8 @@ static int _handle_array(s_p_values_t *v, if (v->handler != NULL) { /* call the handler function */ int rc; - rc = v->handler(&new_ptr, v->type, v->key, value, line); + rc = v->handler(&new_ptr, v->type, v->key, value, + line, leftover); if (rc != 1) return rc == 0 ? 0 : -1; } else { @@ -566,7 +573,7 @@ static int _handle_array(s_p_values_t *v, } static int _handle_boolean(s_p_values_t *v, - const char *value, const char *line) + const char *value, const char *line, char **leftover) { if (v->data_count != 0) { error("%s specified more than once", v->key); @@ -576,7 +583,8 @@ static int _handle_boolean(s_p_values_t *v, if (v->handler != NULL) { /* call the handler function */ int rc; - rc = v->handler(&v->data, v->type, v->key, value, line); + rc = v->handler(&v->data, v->type, v->key, value, + line, leftover); if (rc != 1) return rc == 0 ? 0 : -1; } else { @@ -604,35 +612,46 @@ static int _handle_boolean(s_p_values_t *v, return 1; } + +/* + * IN line: the entire line that currently being parsed + * IN/OUT leftover: leftover is a pointer into the "line" string. + * The incoming leftover point is a pointer to the + * character just after the already parsed key/value pair. + * If the handler for that key parses more of the line, + * it will move the leftover pointer to point to the character + * after it has finished parsing in the line. + */ static void _handle_keyvalue_match(s_p_values_t *v, - const char *value, const char *line) + const char *value, const char *line, + char **leftover) { - /* debug3("key = %s, value = %s, line = \"%s\"", */ - /* v->key, value, line); */ +/* debug3("key = %s, value = %s, line = \"%s\"", */ +/* v->key, value, line); */ switch (v->type) { case S_P_IGNORE: /* do nothing */ break; case S_P_STRING: - _handle_string(v, value, line); + _handle_string(v, value, line, leftover); break; case S_P_LONG: - _handle_long(v, value, line); + _handle_long(v, value, line, leftover); break; case S_P_UINT16: - _handle_uint16(v, value, line); + _handle_uint16(v, value, line, leftover); break; case S_P_UINT32: - _handle_uint32(v, value, line); + _handle_uint32(v, value, line, leftover); break; case S_P_POINTER: - _handle_pointer(v, value, line); + _handle_pointer(v, value, line, leftover); break; case S_P_ARRAY: - _handle_array(v, value, line); + _handle_array(v, value, line, leftover); break; case S_P_BOOLEAN: - _handle_boolean(v, value, line); + _handle_boolean(v, value, line, leftover); break; } } @@ -658,21 +677,22 @@ static int _line_is_space(const char *line) /* * Returns 1 if the line is parsed cleanly, and 0 otherwise. */ -int s_p_parse_line(s_p_hashtbl_t *hashtbl, const char *line) +int s_p_parse_line(s_p_hashtbl_t *hashtbl, const char *line, char **leftover) { char *key, *value; - const char *ptr = line; - char *leftover = (char *)line; + char *ptr = (char *)line; s_p_values_t *p; + char *new_leftover; _keyvalue_regex_init(); - while (_keyvalue_regex(ptr, &key, &value, &leftover) == 0) { + while (_keyvalue_regex(ptr, &key, &value, &new_leftover) == 0) { if ((p = _conf_hashtbl_lookup(hashtbl, key))) { - _handle_keyvalue_match(p, value, line); - ptr = leftover; + _handle_keyvalue_match(p, value, + new_leftover, &new_leftover); + *leftover = ptr = new_leftover; } else { - error("Parsing failed at unrecognized key: %s", key); + error("Parsing error at unrecognized key: %s", key); xfree(key); xfree(value); return 0; @@ -681,12 +701,34 @@ int s_p_parse_line(s_p_hashtbl_t *hashtbl, const char *line) xfree(value); } - if (!_line_is_space(leftover)) { - char *ptr = xstrdup(leftover); - _strip_cr_nl(ptr); - error("Parsing failed at: \"%s\"", ptr); - xfree(ptr); - return 0; + return 1; +} + +/* + * Returns 1 if the line is parsed cleanly, and 0 otherwise. + */ +static int s_p_parse_next_key(s_p_hashtbl_t *hashtbl, + const char *line, char **leftover) +{ + char *key, *value; + s_p_values_t *p; + char *new_leftover; + + _keyvalue_regex_init(); + + if (_keyvalue_regex(line, &key, &value, &new_leftover) == 0) { + if ((p = _conf_hashtbl_lookup(hashtbl, key))) { + _handle_keyvalue_match(p, value, + new_leftover, &new_leftover); + *leftover = new_leftover; + } else { + error("Parsing error at unrecognized key: %s", key); + xfree(key); + xfree(value); + return 0; + } + xfree(key); + xfree(value); } return 1; @@ -696,6 +738,8 @@ int s_p_parse_file(s_p_hashtbl_t *hashtbl, char *filename) { FILE *f; char line[BUFFER_SIZE]; + char *leftover; + int rc = SLURM_SUCCESS; if(!filename) { error("s_p_parse_file: No filename given."); @@ -715,11 +759,20 @@ int s_p_parse_file(s_p_hashtbl_t *hashtbl, char *filename) if (line[0] == '\0') continue; - s_p_parse_line(hashtbl, line); + s_p_parse_next_key(hashtbl, line, &leftover); + + /* Make sure that after parsing only whitespace is left over */ + if (!_line_is_space(leftover)) { + char *ptr = xstrdup(leftover); + _strip_cr_nl(ptr); + error("Parsing error at: \"%s\"", ptr); + xfree(ptr); + rc = SLURM_ERROR; + } } fclose(f); - return SLURM_SUCCESS; + return rc; } /* diff --git a/src/common/parse_config.h b/src/common/parse_config.h index 390b1329d78..d4df73f051b 100644 --- a/src/common/parse_config.h +++ b/src/common/parse_config.h @@ -120,18 +120,21 @@ * a "handler" function and a "destroy" function. The prototypes for each * are available below in the typedef of s_p_options_t. * - * A handler function is given the the "key" string, "value" string, and a - * pointer to the entire "line" on which the key-value pair was found (this is - * the line after the parser has removed comments and concatenated continued - * lines). The handler can transform the value any way it desires, and then - * return a pointer to the newly allocated value data in the "data" pointer. - * The return code from "handler" must be -1 if the value is invalid, 0 if - * the value is valid but no value will be set for "data" (the parser will not - * flag this key as already seen, and the destroy() function will not be - * called during s_p_hashtbl_destroy()), and 1 if "data" is set. - * - * If the "destroy" function is set for a key, and the parser marked a key as - * "seen" during parsing, then it will pass the pointer to the value data + * The "handler" function is given the the "key" string, "value" string, and a + * pointer to the remainder of the "line" on which the key-value pair was found + * (this is the line after the parser has removed comments and concatenated + * continued lines). The handler can transform the value any way it desires, + * and then return a pointer to the newly allocated value data in the "data" + * pointer. The handler shall also return int the "leftover" parameter a + * pointer into the "line" buffer designating the start of any leftover, + * unparsed characters in the string. The return code from "handler" must be + * -1 if the value is invalid, 0 if the value is valid but no value will be set + * for "data" (the parser will not flag this key as already seen, and the + * destroy() function will not be called during s_p_hashtbl_destroy()), + * and 1 if "data" is set. + * + * If the "destroy" function is set for a key, and the parser will mark the key + * as "seen" during parsing, then it will pass the pointer to the value data * to the "destroy" function when s_p_hashtbl_destroy() is called. If * a key was "seen" during parsing, but the "destroy" function is NULL, * s_p_hashtbl_destroy() will call xfree() on the data pointer. @@ -155,7 +158,8 @@ typedef struct conf_file_options { char *key; slurm_parser_enum_t type; int (*handler)(void **data, slurm_parser_enum_t type, - const char *key, const char *value, const char *line); + const char *key, const char *value, + const char *line, char **leftover); void (*destroy)(void *data); } s_p_options_t; @@ -169,7 +173,7 @@ int s_p_parse_file(s_p_hashtbl_t *hashtbl, char *filename); /* * Returns 1 if the line is parsed cleanly, and 0 otherwise. */ -int s_p_parse_line(s_p_hashtbl_t *hashtbl, const char *line); +int s_p_parse_line(s_p_hashtbl_t *hashtbl, const char *line, char **leftover); /* * s_p_get_string diff --git a/src/common/read_config.c b/src/common/read_config.c index d38c037c673..9361e545a0e 100644 --- a/src/common/read_config.c +++ b/src/common/read_config.c @@ -73,7 +73,7 @@ static bool conf_initialized = false; static uint16_t default_slurmd_port; static int parse_slurmd_port(void **dest, slurm_parser_enum_t type, const char *key, const char *value, - const char *line); + const char *line, char **leftover); static s_p_hashtbl_t *default_nodename_tbl; static s_p_hashtbl_t *default_partition_tbl; @@ -98,19 +98,19 @@ static names_ll_t *node_to_host_hashtbl[NAME_HASH_LEN] = {NULL}; static int parse_nodename(void **dest, slurm_parser_enum_t type, const char *key, const char *value, - const char *line); + const char *line, char **leftover); static void destroy_nodename(void *ptr); static int parse_partitionname(void **dest, slurm_parser_enum_t type, const char *key, const char *value, - const char *line); + const char *line, char **leftover); static void destroy_partitionname(void *ptr); static int parse_downnodes(void **dest, slurm_parser_enum_t type, const char *key, const char *value, - const char *line); + const char *line, char **leftover); static void destroy_downnodes(void *ptr); static int defunct_option(void **dest, slurm_parser_enum_t type, const char *key, const char *value, - const char *line); + const char *line, char **leftover); static void validate_and_set_defaults(slurm_ctl_conf_t *conf, s_p_hashtbl_t *hashtbl); @@ -180,75 +180,13 @@ s_p_options_t slurm_conf_options[] = { {"WaitTime", S_P_UINT16}, {"NodeName", S_P_ARRAY, parse_nodename, destroy_nodename}, - /* The following keywords are ignored by this parser and handled - by the NodeName handler */ - {"NodeHostname"}, - {"NodeAddr"}, - {"Feature"}, - {"Port"}, - {"Procs"}, - {"RealMemory"}, - {"Reason"}, - {"State"}, - {"TmpDisk"}, - {"Weight"}, - {"PartitionName", S_P_ARRAY, parse_partitionname, destroy_partitionname}, - /* The following keywords are ignored by this parser and handled - by the PartitionName handler */ - {"AllowGroups"}, - {"Default"}, - {"Hidden"}, - {"MaxTime"}, - {"MaxNodes"}, - {"MinNodes"}, - {"Nodes"}, - {"RootOnly"}, - {"Shared"}, - {"State"}, - {"DownNodes", S_P_ARRAY, parse_downnodes, destroy_downnodes}, - /* "State" and "Reason" are already ignored */ - - {NULL} -}; -static s_p_options_t _nodename_options[] = { - {"NodeName", S_P_STRING}, - {"NodeHostname", S_P_STRING}, - {"NodeAddr", S_P_STRING}, - {"Feature", S_P_STRING}, - {"Port", S_P_UINT16}, - {"Procs", S_P_UINT32}, - {"RealMemory", S_P_UINT32}, - {"Reason", S_P_STRING}, - {"State", S_P_STRING}, - {"TmpDisk", S_P_UINT32}, - {"Weight", S_P_UINT32}, {NULL} }; -static s_p_options_t _partition_options[] = { - {"PartitionName", S_P_STRING}, - {"AllowGroups", S_P_STRING}, - {"Default", S_P_BOOLEAN}, /* YES or NO */ - {"Hidden", S_P_BOOLEAN}, /* YES or NO */ - {"MaxTime", S_P_UINT32}, /* INFINITE or a number */ - {"MaxNodes", S_P_UINT32}, /* INFINITE or a number */ - {"MinNodes", S_P_UINT32}, - {"Nodes", S_P_STRING}, - {"RootOnly", S_P_BOOLEAN}, /* YES or NO */ - {"Shared", S_P_STRING}, /* YES, NO, or FORCE */ - {"State", S_P_BOOLEAN}, /* UP or DOWN */ - {NULL} -}; -static s_p_options_t _downnodes_options[] = { - {"DownNodes", S_P_STRING}, - {"Reason", S_P_STRING}, - {"State", S_P_STRING}, - {NULL} -}; /* * This function works almost exactly the same as the * default S_P_UINT32 handler, except that it also sets the @@ -256,7 +194,7 @@ static s_p_options_t _downnodes_options[] = { */ static int parse_slurmd_port(void **dest, slurm_parser_enum_t type, const char *key, const char *value, - const char *line) + const char *line, char **leftover) { char *endptr; unsigned long num; @@ -291,20 +229,34 @@ static int parse_slurmd_port(void **dest, slurm_parser_enum_t type, static int defunct_option(void **dest, slurm_parser_enum_t type, const char *key, const char *value, - const char *line) + const char *line, char **leftover) { error("The option \"%s\" is defunct, see man slurm.conf.", key); return 0; } static int parse_nodename(void **dest, slurm_parser_enum_t type, - const char *key, const char *value, const char *line) + const char *key, const char *value, + const char *line, char **leftover) { s_p_hashtbl_t *tbl, *dflt; slurm_conf_node_t *n; + static s_p_options_t _nodename_options[] = { + {"NodeHostname", S_P_STRING}, + {"NodeAddr", S_P_STRING}, + {"Feature", S_P_STRING}, + {"Port", S_P_UINT16}, + {"Procs", S_P_UINT32}, + {"RealMemory", S_P_UINT32}, + {"Reason", S_P_STRING}, + {"State", S_P_STRING}, + {"TmpDisk", S_P_UINT32}, + {"Weight", S_P_UINT32}, + {NULL} + }; tbl = s_p_hashtbl_create(_nodename_options); - s_p_parse_line(tbl, line); + s_p_parse_line(tbl, *leftover, leftover); /* s_p_dump_values(tbl, _nodename_options); */ if (strcasecmp(value, "DEFAULT") == 0) { @@ -331,7 +283,7 @@ static int parse_nodename(void **dest, slurm_parser_enum_t type, n = xmalloc(sizeof(slurm_conf_node_t)); dflt = default_nodename_tbl; - s_p_get_string(&n->nodenames, "NodeName", tbl); + n->nodenames = xstrdup(value); if (!s_p_get_string(&n->hostnames, "NodeHostname", tbl)) n->hostnames = xstrdup(n->nodenames); if (!s_p_get_string(&n->addresses, "NodeAddr", tbl)) @@ -408,14 +360,29 @@ int slurm_conf_nodename_array(slurm_conf_node_t **ptr_array[]) } static int parse_partitionname(void **dest, slurm_parser_enum_t type, - const char *key, const char *value, const char *line) + const char *key, const char *value, + const char *line, char **leftover) { s_p_hashtbl_t *tbl, *dflt; slurm_conf_partition_t *p; char *tmp = NULL; + static s_p_options_t _partition_options[] = { + {"AllowGroups", S_P_STRING}, + {"Default", S_P_BOOLEAN}, /* YES or NO */ + {"Hidden", S_P_BOOLEAN}, /* YES or NO */ + {"MaxTime", S_P_UINT32}, /* INFINITE or a number */ + {"MaxNodes", S_P_UINT32}, /* INFINITE or a number */ + {"MinNodes", S_P_UINT32}, + {"Nodes", S_P_STRING}, + {"RootOnly", S_P_BOOLEAN}, /* YES or NO */ + {"Shared", S_P_STRING}, /* YES, NO, or FORCE */ + {"State", S_P_BOOLEAN}, /* UP or DOWN */ + {NULL} + }; + tbl = s_p_hashtbl_create(_partition_options); - s_p_parse_line(tbl, line); + s_p_parse_line(tbl, *leftover, leftover); /* s_p_dump_values(tbl, _partition_options); */ if (strcasecmp(value, "DEFAULT") == 0) { @@ -428,7 +395,7 @@ static int parse_partitionname(void **dest, slurm_parser_enum_t type, p = xmalloc(sizeof(slurm_conf_partition_t)); dflt = default_partition_tbl; - s_p_get_string(&p->name, "PartitionName", tbl); + p->name = xstrdup(value); if (!s_p_get_string(&p->allow_groups, "AllowGroups", tbl)) s_p_get_string(&p->allow_groups, "AllowGroups", dflt); @@ -529,19 +496,24 @@ int slurm_conf_partition_array(slurm_conf_partition_t **ptr_array[]) static int parse_downnodes(void **dest, slurm_parser_enum_t type, const char *key, const char *value, - const char *line) + const char *line, char **leftover) { s_p_hashtbl_t *tbl, *dflt; slurm_conf_downnodes_t *n; + static s_p_options_t _downnodes_options[] = { + {"Reason", S_P_STRING}, + {"State", S_P_STRING}, + {NULL} + }; tbl = s_p_hashtbl_create(_downnodes_options); - s_p_parse_line(tbl, line); + s_p_parse_line(tbl, *leftover, leftover); /* s_p_dump_values(tbl, _downnodes_options); */ n = xmalloc(sizeof(slurm_conf_node_t)); dflt = default_nodename_tbl; - s_p_get_string(&n->nodenames, "DownNodes", tbl); + n->nodenames = xstrdup(value); if (!s_p_get_string(&n->reason, "Reason", tbl)) n->reason = xstrdup("Set in slurm.conf"); diff --git a/src/plugins/select/bluegene/block_allocator/block_allocator.c b/src/plugins/select/bluegene/block_allocator/block_allocator.c index 9661789a049..f0cd525baaa 100644 --- a/src/plugins/select/bluegene/block_allocator/block_allocator.c +++ b/src/plugins/select/bluegene/block_allocator/block_allocator.c @@ -73,9 +73,6 @@ s_p_options_t bg_conf_file_options[] = { {"NodeCardNodeCnt", S_P_UINT16}, {"Numpsets", S_P_UINT16}, {"BPs", S_P_ARRAY, parse_blockreq, destroy_blockreq}, - {"Type", S_P_IGNORE}, - {"Nodecards", S_P_IGNORE}, - {"Quarters", S_P_IGNORE}, {NULL} }; @@ -170,10 +167,9 @@ static void _destroy_geo(void *object); extern int parse_blockreq(void **dest, slurm_parser_enum_t type, const char *key, const char *value, - const char *line) + const char *line, char **leftover) { s_p_options_t block_options[] = { - {"BPs", S_P_STRING}, {"Type", S_P_STRING}, {"Nodecards", S_P_UINT16}, {"Quarters", S_P_UINT16}, @@ -184,10 +180,10 @@ extern int parse_blockreq(void **dest, slurm_parser_enum_t type, blockreq_t *n = NULL; tbl = s_p_hashtbl_create(block_options); - s_p_parse_line(tbl, line); + s_p_parse_line(tbl, *leftover, leftover); n = xmalloc(sizeof(blockreq_t)); - s_p_get_string(&n->block, "BPs", tbl); + n->block = xstrdup(value); s_p_get_string(&tmp, "Type", tbl); if (!tmp || !strcasecmp(tmp,"TORUS")) diff --git a/src/plugins/select/bluegene/block_allocator/block_allocator.h b/src/plugins/select/bluegene/block_allocator/block_allocator.h index cb04ca63688..846b0849953 100644 --- a/src/plugins/select/bluegene/block_allocator/block_allocator.h +++ b/src/plugins/select/bluegene/block_allocator/block_allocator.h @@ -195,7 +195,7 @@ extern s_p_options_t bg_conf_file_options[]; extern int parse_blockreq(void **dest, slurm_parser_enum_t type, const char *key, const char *value, - const char *line); + const char *line, char **leftover); extern void destroy_blockreq(void *ptr); -- GitLab