From: Alan T. DeKok Date: Mon, 17 Feb 2025 12:27:08 +0000 (-0500) Subject: rearrange to make name and parent available to validation routines X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f15d5dfa67af1b90b8039050d850b2cca532f06d;p=thirdparty%2Ffreeradius-server.git rearrange to make name and parent available to validation routines so that the type / flag validation routines have more information with which to make their decisions. --- diff --git a/src/lib/util/dict_tokenize.c b/src/lib/util/dict_tokenize.c index c9ae3279f7f..f7b1f3e90c2 100644 --- a/src/lib/util/dict_tokenize.c +++ b/src/lib/util/dict_tokenize.c @@ -960,11 +960,11 @@ static int dict_set_value_attr(dict_tokenize_ctx_t *dctx, fr_dict_attr_t *da) } static int dict_read_process_common(dict_tokenize_ctx_t *dctx, fr_dict_attr_t **da_p, - char const *name, + fr_dict_attr_t const *parent, char const *name, char const *type_name, char *flag_name, fr_dict_attr_flags_t const *base_flags) { - fr_dict_attr_t *da; + fr_dict_attr_t *da, *to_free = NULL; /* * Dictionaries need to have real names, not shitty ones. @@ -980,13 +980,22 @@ static int dict_read_process_common(dict_tokenize_ctx_t *dctx, fr_dict_attr_t ** */ if (!*da_p) { da = dict_attr_alloc_null(dctx->dict->pool, dctx->dict->proto); - if (unlikely(da == NULL)) return -1; + if (unlikely(!da)) return -1; + to_free = da; + } else { da = *da_p; } dict_attr_location_set(dctx, da); da->dict = dctx->dict; + /* + * Set some fields to be friendlier to the type / flag + * parsing and validation routines. + */ + da->parent = parent; + da->name = name; + /* * Set the attribute flags from the base flags. */ @@ -997,10 +1006,16 @@ static int dict_read_process_common(dict_tokenize_ctx_t *dctx, fr_dict_attr_t ** */ if (dict_process_type_field(dctx, type_name, &da) < 0) { error: - talloc_free(da); + talloc_free(to_free); return -1; } + /* + * Clear the temporary parent pointer. + */ + da->parent = NULL; + if (unlikely(dict_attr_parent_init(&da, parent) < 0)) goto error; + /* * Parse optional flags. We pass in the partially allocated * attribute so that flags can be set directly. @@ -1010,6 +1025,8 @@ static int dict_read_process_common(dict_tokenize_ctx_t *dctx, fr_dict_attr_t ** */ if (flag_name) if (dict_process_flag_field(dctx, flag_name, &da) < 0) goto error; + da->name = NULL; /* the real name will be a talloc'd chunk */ + *da_p = da; return 0; } @@ -1268,6 +1285,17 @@ static int dict_read_process_attribute(dict_tokenize_ctx_t *dctx, char **argv, i set_relative_attr = false; } + if (!fr_cond_assert(parent)) return -1; /* Should have provided us with a parent */ + + /* + * Members of a 'struct' MUST use MEMBER, not ATTRIBUTE. + */ + if (parent->type == FR_TYPE_STRUCT) { + fr_strerror_printf("Member %s of ATTRIBUTE %s type 'struct' MUST use the \"MEMBER\" keyword", + argv[0], parent->name); + return -1; + } + da = dict_attr_alloc_null(dctx->dict->pool, dctx->dict->proto); if (unlikely(!da)) return -1; @@ -1287,7 +1315,7 @@ static int dict_read_process_attribute(dict_tokenize_ctx_t *dctx, char **argv, i return -1; } - if (dict_read_process_common(dctx, &da, argv[0], argv[2], + if (dict_read_process_common(dctx, &da, parent, argv[0], argv[2], (argc > 3) ? argv[3] : NULL, base_flags) < 0) { goto error; } @@ -1298,22 +1326,6 @@ static int dict_read_process_attribute(dict_tokenize_ctx_t *dctx, char **argv, i } - if (!fr_cond_assert(parent)) goto error; /* Should have provided us with a parent */ - - /* - * Members of a 'struct' MUST use MEMBER, not ATTRIBUTE. - */ - if (parent->type == FR_TYPE_STRUCT) { - fr_strerror_printf("Member %s of ATTRIBUTE %s type 'struct' MUST use the \"MEMBER\" keyword", - argv[0], parent->name); - goto error; - } - - /* - * Set the parent we just determined... - */ - if (unlikely(dict_attr_parent_init(&da, parent) < 0)) goto error; - #ifdef WITH_DICTIONARY_WARNINGS /* * Hack to help us discover which vendors have illegal @@ -1333,7 +1345,9 @@ static int dict_read_process_attribute(dict_tokenize_ctx_t *dctx, char **argv, i /* * Set the attribute name */ - if (unlikely(dict_attr_finalise(&da, argv[0]) < 0)) goto error; + if (unlikely(dict_attr_finalise(&da, argv[0]) < 0)) { + goto error; + } /* * Check to see if this is a duplicate attribute @@ -1659,7 +1673,26 @@ static int dict_read_process_define(dict_tokenize_ctx_t *dctx, char **argv, int return -1; } - if (dict_read_process_common(dctx, &da, argv[0], argv[1], + frame = dict_dctx_unwind(dctx); + if (!fr_cond_assert(frame && frame->da)) return -1; /* Should have provided us with a parent */ + + parent = frame->da; + + /* + * Members of a 'struct' MUST use MEMBER, not ATTRIBUTE. + */ + if (parent->type == FR_TYPE_STRUCT) { + fr_strerror_printf("Member %s of parent %s type 'struct' MUST use the \"MEMBER\" keyword", + argv[0], parent->name); + return -1; + } + + /* + * We don't set the attribute number before parsing the + * type and flags. The number is chosen internally, and + * no one should depend on it. + */ + if (dict_read_process_common(dctx, &da, parent, argv[0], argv[1], (argc > 2) ? argv[2] : NULL, base_flags) < 0) { return -1; } @@ -1684,20 +1717,6 @@ static int dict_read_process_define(dict_tokenize_ctx_t *dctx, char **argv, int goto error; } - frame = dict_dctx_unwind(dctx); - if (!fr_cond_assert(frame && frame->da)) goto error; /* Should have provided us with a parent */ - - parent = frame->da; - - /* - * Members of a 'struct' MUST use MEMBER, not ATTRIBUTE. - */ - if (parent->type == FR_TYPE_STRUCT) { - fr_strerror_printf("Member %s of parent %s type 'struct' MUST use the \"MEMBER\" keyword", - argv[0], parent->name); - goto error; - } - #ifdef STATIC_ANALYZER if (!dctx->dict) goto error; #endif @@ -1708,8 +1727,6 @@ static int dict_read_process_define(dict_tokenize_ctx_t *dctx, char **argv, int */ da->flags.name_only = true; - if (unlikely(dict_attr_parent_init(&da, parent) < 0)) goto error; - /* * Add an attribute number now so the allocations occur in order */ @@ -2083,7 +2100,26 @@ static int dict_read_process_member(dict_tokenize_ctx_t *dctx, char **argv, int return -1; } - if (dict_read_process_common(dctx, &da, argv[0], argv[1], + /* + * Check if the parent 'struct' is fixed size. And if + * so, complain if we're adding a variable sized member. + */ + if (dctx->stack[dctx->stack_depth].struct_is_closed) { + fr_strerror_printf("Cannot add MEMBER to 'struct' %s after a variable sized member %s", + dctx->stack[dctx->stack_depth].da->name, + dctx->stack[dctx->stack_depth].struct_is_closed->name); + return -1; + } + + /* + * We don't set the attribute number before parsing the + * type and flags. The number is chosen internally, and + * no one should depend on it. + * + * Although _arguably_, it may be useful to know which + * field this is, 0..N? + */ + if (dict_read_process_common(dctx, &da, dctx->stack[dctx->stack_depth].da, argv[0], argv[1], (argc > 2) ? argv[2] : NULL, base_flags) < 0) { return -1; } @@ -2098,7 +2134,7 @@ static int dict_read_process_member(dict_tokenize_ctx_t *dctx, char **argv, int da->flags.is_known_width |= dctx->stack[dctx->stack_depth].da->flags.is_known_width; /* - * Double check bit field magic + * Double check any bit field magic */ if (dctx->stack[dctx->stack_depth].member_num > 0) { fr_dict_attr_t const *previous; @@ -2133,17 +2169,6 @@ static int dict_read_process_member(dict_tokenize_ctx_t *dctx, char **argv, int } } - /* - * Check if the parent 'struct' is fixed size. And if - * so, complain if we're adding a variable sized member. - */ - if (dctx->stack[dctx->stack_depth].struct_is_closed) { - fr_strerror_printf("Cannot add MEMBER to 'struct' %s after a variable sized member %s", - dctx->stack[dctx->stack_depth].da->name, - dctx->stack[dctx->stack_depth].struct_is_closed->name); - goto error; - } - /* * Ensure that no previous child has "key" or "length" set. */ @@ -2172,7 +2197,6 @@ static int dict_read_process_member(dict_tokenize_ctx_t *dctx, char **argv, int } } - if (unlikely(dict_attr_parent_init(&da, dctx->stack[dctx->stack_depth].da) < 0)) goto error; if (unlikely(dict_attr_num_init(da, ++dctx->stack[dctx->stack_depth].member_num) < 0)) goto error; if (unlikely(dict_attr_finalise(&da, argv[0]) < 0)) goto error;