Skip to content

Commit

Permalink
Split dp_option_inherit() into two functions
Browse files Browse the repository at this point in the history
dp_option_inherit() previously included both option matching logic and option
inheritance logic.  Code that only needed the inheritance logic generated a
dummy option list to bypass the matching logic.

Eliminate the need for dummy option lists by moving the matching logic into a
separate dp_option_inherit_match() function.

Reviewed-by: Pavel Březina <[email protected]>
Reviewed-by: Tomáš Halman <[email protected]>
  • Loading branch information
PaulSD authored and pbrezina committed Oct 3, 2022
1 parent 74be536 commit 6e3d2d7
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 37 deletions.
19 changes: 8 additions & 11 deletions src/providers/ad/ad_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -1645,20 +1645,18 @@ ad_user_conn_list(TALLOC_CTX *mem_ctx,
}

errno_t ad_inherit_opts_if_needed(struct dp_option *parent_opts,
struct dp_option *suddom_opts,
struct dp_option *subdom_opts,
struct confdb_ctx *cdb,
const char *subdom_conf_path,
int opt_id)
{
int ret;
const char *parent_val = NULL;
char *dummy = NULL;
char *option_list[2] = { NULL, NULL };
bool is_default = true;
char *dummy = NULL;

switch (parent_opts[opt_id].type) {
case DP_OPT_STRING:
parent_val = dp_opt_get_cstring(parent_opts, opt_id);
is_default = (dp_opt_get_cstring(parent_opts, opt_id) == NULL);
break;
case DP_OPT_BOOL:
/* For booleans it is hard to say if the option is set or not since
Expand All @@ -1667,13 +1665,13 @@ errno_t ad_inherit_opts_if_needed(struct dp_option *parent_opts,
* case the sub-domain option would either be the default as well or
* manully set and in both cases we do not have to change it. */
is_default = (parent_opts[opt_id].val.boolean
== parent_opts[opt_id].def_val.boolean);
== parent_opts[opt_id].def_val.boolean);
break;
default:
DEBUG(SSSDBG_TRACE_FUNC, "Unsupported type, skipping.\n");
}

if (parent_val != NULL || !is_default) {
if (!is_default) {
ret = confdb_get_string(cdb, NULL, subdom_conf_path,
parent_opts[opt_id].opt_name, NULL, &dummy);
if (ret != EOK) {
Expand All @@ -1684,10 +1682,9 @@ errno_t ad_inherit_opts_if_needed(struct dp_option *parent_opts,
if (dummy == NULL) {
DEBUG(SSSDBG_CONF_SETTINGS,
"Option [%s] is set in parent domain but not set for "
"sub-domain trying to set it to [%s].\n",
parent_opts[opt_id].opt_name, parent_val);
option_list[0] = discard_const(parent_opts[opt_id].opt_name);
dp_option_inherit(option_list, opt_id, parent_opts, suddom_opts);
"sub-domain, inheriting it from parent.\n",
parent_opts[opt_id].opt_name);
dp_option_inherit(opt_id, parent_opts, subdom_opts);
}
}

Expand Down
8 changes: 6 additions & 2 deletions src/providers/data_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,12 @@ struct dp_option {

#define DP_OPTION_TERMINATOR { NULL, 0, NULL_STRING, NULL_STRING }

void dp_option_inherit(char **inherit_opt_list,
int option,
void dp_option_inherit_match(char **inherit_opt_list,
int option,
struct dp_option *parent_opts,
struct dp_option *subdom_opts);

void dp_option_inherit(int option,
struct dp_option *parent_opts,
struct dp_option *subdom_opts);

Expand Down
20 changes: 14 additions & 6 deletions src/providers/data_provider_opts.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,11 @@
#include "data_provider.h"

/* =Copy-Option-From-Subdomain-If-Allowed================================= */
void dp_option_inherit(char **inherit_opt_list,
int option,
struct dp_option *parent_opts,
struct dp_option *subdom_opts)
void dp_option_inherit_match(char **inherit_opt_list,
int option,
struct dp_option *parent_opts,
struct dp_option *subdom_opts)
{
errno_t ret;
bool inherit_option;

inherit_option = string_in_list(parent_opts[option].opt_name,
Expand All @@ -39,8 +38,17 @@ void dp_option_inherit(char **inherit_opt_list,
return;
}

dp_option_inherit(option, parent_opts, subdom_opts);
}

void dp_option_inherit(int option,
struct dp_option *parent_opts,
struct dp_option *subdom_opts)
{
errno_t ret;

DEBUG(SSSDBG_CONF_SETTINGS,
"Will inherit option %s\n", parent_opts[option].opt_name);
"Inheriting option %s\n", parent_opts[option].opt_name);
switch (parent_opts[option].type) {
case DP_OPT_NUMBER:
ret = dp_opt_set_int(subdom_opts,
Expand Down
8 changes: 4 additions & 4 deletions src/providers/ldap/sdap.c
Original file line number Diff line number Diff line change
Expand Up @@ -278,10 +278,10 @@ static void sdap_inherit_basic_options(char **inherit_opt_list,
int i;

for (i = 0; inherit_options[i] != SDAP_OPTS_BASIC; i++) {
dp_option_inherit(inherit_opt_list,
inherit_options[i],
parent_opts,
subdom_opts);
dp_option_inherit_match(inherit_opt_list,
inherit_options[i],
parent_opts,
subdom_opts);
}
}

Expand Down
28 changes: 14 additions & 14 deletions src/tests/cmocka/test_dp_opts.c
Original file line number Diff line number Diff line change
Expand Up @@ -423,33 +423,33 @@ void opt_test_inherit(void **state)
assert_int_equal(ret, EOK);
assert_defaults(opts);

dp_option_inherit(NULL, OPT_STRING_NODEFAULT,
opts, opts_copy);
dp_option_inherit_match(NULL, OPT_STRING_NODEFAULT,
opts, opts_copy);
s = dp_opt_get_string(opts_copy, OPT_STRING_NODEFAULT);
assert_null(s);

/* string */
assert_nondefault_string_empty(opts_copy);
set_nondefault_string(opts);
dp_option_inherit(discard_const(sd_inherit_match),
OPT_STRING_NODEFAULT,
opts, opts_copy);
dp_option_inherit_match(discard_const(sd_inherit_match),
OPT_STRING_NODEFAULT,
opts, opts_copy);
check_nondefault_string(opts_copy);

/* blob */
assert_nondefault_blob_empty(opts_copy);
set_nondefault_blob(opts);
dp_option_inherit(discard_const(sd_inherit_match),
OPT_BLOB_NODEFAULT,
opts, opts_copy);
dp_option_inherit_match(discard_const(sd_inherit_match),
OPT_BLOB_NODEFAULT,
opts, opts_copy);
check_nondefault_blob(opts_copy);

/* number */
assert_nondefault_int_notset(opts_copy);
set_nondefault_int(opts);
dp_option_inherit(discard_const(sd_inherit_match),
OPT_INT_NODEFAULT,
opts, opts_copy);
dp_option_inherit_match(discard_const(sd_inherit_match),
OPT_INT_NODEFAULT,
opts, opts_copy);
assert_nondefault_int_set(opts_copy);

/* bool */
Expand All @@ -458,9 +458,9 @@ void opt_test_inherit(void **state)
ret = dp_opt_set_bool(opts, OPT_BOOL_TRUE, false);
assert_int_equal(ret, EOK);

dp_option_inherit(discard_const(sd_inherit_match),
OPT_BOOL_TRUE,
opts, opts_copy);
dp_option_inherit_match(discard_const(sd_inherit_match),
OPT_BOOL_TRUE,
opts, opts_copy);

assert_false(dp_opt_get_bool(opts_copy, OPT_BOOL_TRUE));
}
Expand Down

0 comments on commit 6e3d2d7

Please sign in to comment.