From: Alan T. DeKok Date: Fri, 19 Feb 2021 14:12:53 +0000 (-0500) Subject: add section / pair / data in alloc functions, not elsewhere X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=e04e82ddf79e832c7b797552ea9aa519ae6d5586;p=thirdparty%2Ffreeradius-server.git add section / pair / data in alloc functions, not elsewhere on review, most of the calls to cf_section_add() were wrong. the child section was already added to the parent section by _cf_section_alloc(). There were many, many, calls to cf_item_add() or cf_section_add() which resulted in the section being added twice. However, cf_pair_alloc() and cf_data_alloc() did not add the item to the parent. In the interest of less code and fewer errors, we've removed cf_section_add() and cf_pair_add(). The calls to cf_item_add() are now done only from the alloc functions. We still have many calls to cf_data_add(), because that function is in fact allocating *and* adding the CONF_DATA items. --- diff --git a/src/bin/unit_test_module.c b/src/bin/unit_test_module.c index 350e1382f6d..25f702ec55a 100644 --- a/src/bin/unit_test_module.c +++ b/src/bin/unit_test_module.c @@ -106,25 +106,12 @@ static RADCLIENT *client_alloc(TALLOC_CTX *ctx, char const *ip, char const *name cs = cf_section_alloc(ctx, NULL, "client", name); cp = cf_pair_alloc(cs, "ipaddr", ip, T_OP_EQ, T_BARE_WORD, T_BARE_WORD); - cf_pair_add(cs, cp); - cp = cf_pair_alloc(cs, "secret", "supersecret", T_OP_EQ, T_BARE_WORD, T_DOUBLE_QUOTED_STRING); - cf_pair_add(cs, cp); - cp = cf_pair_alloc(cs, "nas_type", "test", T_OP_EQ, T_BARE_WORD, T_DOUBLE_QUOTED_STRING); - cf_pair_add(cs, cp); - cp = cf_pair_alloc(cs, "shortname", "test", T_OP_EQ, T_BARE_WORD, T_DOUBLE_QUOTED_STRING); - cf_pair_add(cs, cp); - cp = cf_pair_alloc(cs, "groups", "foo", T_OP_EQ, T_BARE_WORD, T_DOUBLE_QUOTED_STRING); - cf_pair_add(cs, cp); - cp = cf_pair_alloc(cs, "groups", "bar", T_OP_EQ, T_BARE_WORD, T_DOUBLE_QUOTED_STRING); - cf_pair_add(cs, cp); - cp = cf_pair_alloc(cs, "groups", "baz", T_OP_EQ, T_BARE_WORD, T_DOUBLE_QUOTED_STRING); - cf_pair_add(cs, cp); client = client_afrom_cs(ctx, cs, NULL); if (!client) { diff --git a/src/lib/server/cf_file.c b/src/lib/server/cf_file.c index 8131469ea73..73596b1ce2e 100644 --- a/src/lib/server/cf_file.c +++ b/src/lib/server/cf_file.c @@ -321,7 +321,6 @@ char const *cf_expand_variables(char const *cf, int lineno, cf_filename_set(subcs, ci->filename); cf_lineno_set(subcs, ci->lineno); - cf_item_add(outer_cs, &(subcs->item)); ptr = next + 1; @@ -436,7 +435,6 @@ static bool cf_template_merge(CONF_SECTION *cs, CONF_SECTION const *template) cf_filename_set(cp2, cp1->item.filename); cf_lineno_set(cp2, cp1->item.lineno); - cf_item_add(cs, &(cp2->item)); continue; } @@ -469,7 +467,6 @@ static bool cf_template_merge(CONF_SECTION *cs, CONF_SECTION const *template) cf_filename_set(subcs2, subcs1->item.filename); cf_lineno_set(subcs2, subcs1->item.lineno); - cf_item_add(cs, &(subcs2->item)); continue; } @@ -1565,7 +1562,6 @@ static int add_pair(CONF_SECTION *parent, char const *attr, char const *value, cf_filename_set(cp, filename); cf_lineno_set(cp, lineno); cp->pass2 = pass2; - cf_item_add(parent, &(cp->item)); cd = cf_data_find(CF_TO_ITEM(parent), CONF_PARSER, attr); if (!cd) return 0; @@ -2326,8 +2322,6 @@ int cf_file_read(CONF_SECTION *cs, char const *filename) p = strrchr(cp->value, FR_DIR_SEP); if (p) *p = '\0'; - cf_item_add(cs, &(cp->item)); - MEM(tree = rbtree_talloc_alloc(cs, cf_file_t, node, _inode_cmp, NULL, 0)); cf_data_add(cs, tree, "filename", false); diff --git a/src/lib/server/cf_parse.c b/src/lib/server/cf_parse.c index cd53539518a..f903ee5f820 100644 --- a/src/lib/server/cf_parse.c +++ b/src/lib/server/cf_parse.c @@ -831,7 +831,6 @@ static int CC_HINT(nonnull(4,5)) cf_pair_parse_internal(TALLOC_CTX *ctx, void *o ret = func(value_ctx, entry, base, cf_pair_to_item(cp), rule); if (ret < 0) { talloc_free(array); - talloc_free(dflt_cp); return -1; } cp->parsed = true; @@ -880,23 +879,10 @@ static int CC_HINT(nonnull(4,5)) cf_pair_parse_internal(TALLOC_CTX *ctx, void *o } ret = func(ctx, out, base, cf_pair_to_item(cp), rule); - if (ret < 0) { - talloc_free(dflt_cp); - return -1; - } + if (ret < 0) return -1; cp->parsed = true; } - /* - * If we created a default cp and succeeded - * in parsing the dflt value, add the new - * cp to the enclosing section. - */ - if (dflt_cp) { - cf_item_add(cs, &(dflt_cp->item)); - return 1; - } - return 0; } diff --git a/src/lib/server/cf_util.c b/src/lib/server/cf_util.c index 1f7717d82c7..9ab8e077102 100644 --- a/src/lib/server/cf_util.c +++ b/src/lib/server/cf_util.c @@ -854,7 +854,6 @@ CONF_SECTION *cf_section_dup(TALLOC_CTX *ctx, CONF_SECTION *parent, CONF_SECTION talloc_free(new); return NULL; } - cf_section_add(new, subcs); break; case CONF_ITEM_PAIR: @@ -863,7 +862,6 @@ CONF_SECTION *cf_section_dup(TALLOC_CTX *ctx, CONF_SECTION *parent, CONF_SECTION talloc_free(new); return NULL; } - cf_pair_add(new, cp); break; case CONF_ITEM_DATA: /* Skip data */ @@ -877,17 +875,6 @@ CONF_SECTION *cf_section_dup(TALLOC_CTX *ctx, CONF_SECTION *parent, CONF_SECTION return new; } -/** Add a section as a child of another section - * - * @param[in] parent section we're adding to. - * @param[in] cs we're adding. - * - * @hidecallergraph - */ -void cf_section_add(CONF_SECTION *parent, CONF_SECTION *cs) -{ - cf_item_add(parent, &(cs->item)); -} /** Return the next child that's a #CONF_SECTION * @@ -1122,6 +1109,7 @@ CONF_PAIR *cf_pair_alloc(CONF_SECTION *parent, char const *attr, char const *val if (!cp->value) goto error; } + cf_item_add(parent, &(cp->item)); return cp; } @@ -1185,15 +1173,6 @@ int cf_pair_replace(CONF_SECTION *cs, CONF_PAIR *cp, char const *value) return 0; } -/** Add a configuration pair to a section - * - * @param[in] parent section to add pair to. - * @param[in] cp to add. - */ -void cf_pair_add(CONF_SECTION *parent, CONF_PAIR *cp) -{ - cf_item_add(parent, cf_pair_to_item(cp)); -} /** Mark a pair as parsed * @@ -1430,6 +1409,7 @@ static CONF_DATA *cf_data_alloc(CONF_ITEM *parent, void const *data, char const talloc_set_destructor(cd, _cd_free); } + cf_item_add(parent, cd); return cd; } @@ -1547,7 +1527,6 @@ CONF_DATA const *_cf_data_add(CONF_ITEM *ci, void const *data, char const *name, cd->is_talloced = true; cf_filename_set(cd, filename); cf_lineno_set(cd, lineno); - cf_item_add(ci, cd); return cd; } @@ -1592,7 +1571,6 @@ CONF_DATA const *_cf_data_add_static(CONF_ITEM *ci, void const *data, char const cd->is_talloced = false; cf_filename_set(cd, filename); cf_lineno_set(cd, lineno); - cf_item_add(ci, cd); return cd; } diff --git a/src/lib/server/cf_util.h b/src/lib/server/cf_util.h index efd580358d6..6ca8e2f3f1b 100644 --- a/src/lib/server/cf_util.h +++ b/src/lib/server/cf_util.h @@ -140,7 +140,6 @@ CONF_SECTION *_cf_section_alloc(TALLOC_CTX *ctx, CONF_SECTION *parent, char const *filename, int lineno); CONF_SECTION *cf_section_dup(TALLOC_CTX *ctx, CONF_SECTION *parent, CONF_SECTION const *cs, char const *name1, char const *name2, bool copy_meta); -void cf_section_add(CONF_SECTION *parent, CONF_SECTION *cs); /** @hidecallergraph */ CONF_SECTION *cf_section_next(CONF_SECTION const *cs, CONF_SECTION const *prev); /** @hidecallergraph */ @@ -170,7 +169,6 @@ CONF_PAIR *cf_pair_alloc(CONF_SECTION *parent, char const *attr, char const *val fr_token_t op, fr_token_t lhs_type, fr_token_t rhs_type); CONF_PAIR *cf_pair_dup(CONF_SECTION *parent, CONF_PAIR *cp); int cf_pair_replace(CONF_SECTION *cs, CONF_PAIR *cp, char const *value); -void cf_pair_add(CONF_SECTION *parent, CONF_PAIR *cp); void cf_pair_mark_parsed(CONF_PAIR *cp); CONF_PAIR *cf_pair_next(CONF_SECTION const *cs, CONF_PAIR const *prev); CONF_PAIR *cf_pair_find(CONF_SECTION const *cs, char const *name); diff --git a/src/lib/server/client.c b/src/lib/server/client.c index 0119c44eb76..c17a98c525d 100644 --- a/src/lib/server/client.c +++ b/src/lib/server/client.c @@ -634,7 +634,6 @@ int client_map_section(CONF_SECTION *out, CONF_SECTION const *map, client_value_ cc = cf_section_find(out, cf_section_name1(cs), cf_section_name2(cs)); if (!cc) { cc = cf_section_alloc(out, out, cf_section_name1(cs), cf_section_name2(cs)); - cf_section_add(out, cc); if (!cc) return -1; } @@ -1025,7 +1024,6 @@ RADCLIENT *client_afrom_request(TALLOC_CTX *ctx, request_t *request) } RDEBUG2("%s = %s", cf_pair_attr(cp), cf_pair_value(cp)); - cf_pair_add(cs, cp); } REXDENT(); diff --git a/src/lib/server/dependency.c b/src/lib/server/dependency.c index cf87f6fe618..56adb3e2002 100644 --- a/src/lib/server/dependency.c +++ b/src/lib/server/dependency.c @@ -290,7 +290,6 @@ int dependency_feature_add(CONF_SECTION *cs, char const *name, bool enabled) cp = cf_pair_alloc(cs, name, enabled ? "yes" : "no", T_OP_EQ, T_BARE_WORD, T_BARE_WORD); if (!cp) return -1; - cf_pair_add(cs, cp); } return 0; @@ -333,7 +332,6 @@ int dependency_version_number_add(CONF_SECTION *cs, char const *name, char const cp = cf_pair_alloc(cs, name, version, T_OP_EQ, T_BARE_WORD, T_SINGLE_QUOTED_STRING); if (!cp) return -1; - cf_pair_add(cs, cp); } else { WARN("Replacing user version.%s (%s) with %s", name, cf_pair_value(old), version); diff --git a/src/lib/server/main_config.c b/src/lib/server/main_config.c index 056a4dcf21f..3858e8d577e 100644 --- a/src/lib/server/main_config.c +++ b/src/lib/server/main_config.c @@ -1005,7 +1005,6 @@ do {\ } } else { MEM(cp = cf_pair_alloc(cs, "name", config->name, T_OP_EQ, T_BARE_WORD, T_DOUBLE_QUOTED_STRING)); - cf_pair_add(cs, cp); } } diff --git a/src/listen/radius/proto_radius.c b/src/listen/radius/proto_radius.c index 8eeb318e64b..838936be161 100644 --- a/src/listen/radius/proto_radius.c +++ b/src/listen/radius/proto_radius.c @@ -175,7 +175,6 @@ static int transport_parse(TALLOC_CTX *ctx, void *out, UNUSED void *parent, CONF */ if (!transport_cs) { transport_cs = cf_section_alloc(listen_cs, listen_cs, name, NULL); - cf_section_add(listen_cs, transport_cs); inst->io.app_io_conf = transport_cs; } diff --git a/src/listen/tacacs/proto_tacacs.c b/src/listen/tacacs/proto_tacacs.c index e22b3ca338f..35e118cb52a 100644 --- a/src/listen/tacacs/proto_tacacs.c +++ b/src/listen/tacacs/proto_tacacs.c @@ -151,7 +151,6 @@ static int transport_parse(TALLOC_CTX *ctx, void *out, UNUSED void *parent, CONF */ if (!transport_cs) { transport_cs = cf_section_alloc(listen_cs, listen_cs, name, NULL); - cf_section_add(listen_cs, transport_cs); inst->io.app_io_conf = transport_cs; } diff --git a/src/modules/proto_ldap_sync/proto_ldap_sync.c b/src/modules/proto_ldap_sync/proto_ldap_sync.c index d84e39f6e25..4b0de34960a 100644 --- a/src/modules/proto_ldap_sync/proto_ldap_sync.c +++ b/src/modules/proto_ldap_sync/proto_ldap_sync.c @@ -785,9 +785,7 @@ static RADCLIENT *proto_ldap_fake_client_alloc(proto_ldap_inst_t *inst) cs = cf_section_alloc(NULL, NULL, "client", "ldap"); cp = cf_pair_alloc(cs, "ipaddr", fr_inet_ntop(buffer, sizeof(buffer), &inst->dst_ipaddr), T_OP_EQ, T_BARE_WORD, T_BARE_WORD); - cf_pair_add(cs, cp); cp = cf_pair_alloc(cs, "secret", "fake", T_OP_EQ, T_BARE_WORD, T_BARE_WORD); - cf_pair_add(cs, cp); client = client_afrom_cs(inst, cs, NULL); if (!client) { diff --git a/src/modules/rlm_redis_ippool/rlm_redis_ippool_tool.c b/src/modules/rlm_redis_ippool/rlm_redis_ippool_tool.c index e627ca4fd05..b067b1207d7 100644 --- a/src/modules/rlm_redis_ippool/rlm_redis_ippool_tool.c +++ b/src/modules/rlm_redis_ippool/rlm_redis_ippool_tool.c @@ -1340,7 +1340,6 @@ do { \ ERROR("Failed creating server pair"); fr_exit_now(EXIT_FAILURE); } - cf_pair_add(conf->cs, cp); /* * Unescape sequences in the pool name @@ -1386,17 +1385,14 @@ do { \ cp = cf_pair_find(pool_cs, "start"); if (!cp) { cp = cf_pair_alloc(pool_cs, "start", "0", T_OP_EQ, T_BARE_WORD, T_BARE_WORD); - cf_pair_add(pool_cs, cp); } cp = cf_pair_find(pool_cs, "spare"); if (!cp) { cp = cf_pair_alloc(pool_cs, "spare", "0", T_OP_EQ, T_BARE_WORD, T_BARE_WORD); - cf_pair_add(pool_cs, cp); } cp = cf_pair_find(pool_cs, "min"); if (!cp) { cp = cf_pair_alloc(pool_cs, "min", "0", T_OP_EQ, T_BARE_WORD, T_BARE_WORD); - cf_pair_add(pool_cs, cp); } if (driver_init(conf, conf->cs, &conf->driver) < 0) {