Skip to content

Commit

Permalink
Fix RlistEqual comparison on lists of different lengths
Browse files Browse the repository at this point in the history
The original issue that leads to this is the fact that the cache
for execresult and execresult_as_data were mixed. This is caused by two
separate issues:

* The function cache uses the args list as key and discards the function
  itself. This means different function with the same args are considered
  identical, and cache is reused.
* The args are passed as an Rlist, and the used Rlist comparison ignores the
  additional items of the longest list when comparing two lists of
  different lengths, leading to treating execresult and execresult_as_data
  as identical when using the same command and shell args.

This PR only fixes the specific case of execresult, but leaves other
function cache issues (e.g. host2ip vs. ip2host could be confused).
  • Loading branch information
amousset committed Sep 12, 2023
1 parent 57745c7 commit d5a7372
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 2 deletions.
4 changes: 2 additions & 2 deletions libpromises/rlist.c
Original file line number Diff line number Diff line change
Expand Up @@ -1657,8 +1657,8 @@ bool RlistEqual(const Rlist *list1, const Rlist *list2)
assert(rp1->val.item == NULL && rp2->val.item == NULL);
}
}

return true;
// return false if lengths are different
return (rp1 == NULL && rp2 == NULL);
}

bool RlistEqual_untyped(const void *list1, const void *list2)
Expand Down
41 changes: 41 additions & 0 deletions tests/acceptance/01_vars/04_containers/execresult_and_as_data.cf
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
#######################################################
#
# Test the ability to call the same command with execresult and execresult_as_data
#
#######################################################

body common control
{
inputs => { "../../default.cf.sub" };
bundlesequence => { default("$(this.promise_filename)") };
version => "1.0";
}

#######################################################


bundle agent init
{
}

#######################################################

bundle agent test
{
vars:
!windows::
"res1" string => execresult("echo test", "useshell");
"res2" data => execresult_as_data("echo test", "useshell", "stdout");
windows::
"res1" string => execresult("echo test", "powershell");
"res2" data => execresult_as_data("echo test", "powershell", "stdout");
}


#######################################################

bundle agent check
{
methods:
"any" usebundle => dcs_check_strcmp("${test.res1}", "${test.res2[output]}", "$(this.promise_filename)", "no");
}
18 changes: 18 additions & 0 deletions tests/unit/rlist_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,23 @@ static void test_length(void)
RlistDestroy(list);
}

static void test_equality(void)
{
Rlist *list1 = RlistFromSplitString("a,b,c", ',');
Rlist *list2 = RlistFromSplitString("a,b,c", ',');
Rlist *list3 = RlistFromSplitString("z,b,c", ',');
Rlist *list4 = RlistFromSplitString("a,b,c,d", ',');

assert_true(RlistEqual(list1, list2));
assert_false(RlistEqual(list1, list3));
assert_false(RlistEqual(list1, list4));

RlistDestroy(list1);
RlistDestroy(list2);
RlistDestroy(list3);
RlistDestroy(list4);
}

static void test_prepend_scalar_idempotent(void)
{
Rlist *list = NULL;
Expand Down Expand Up @@ -750,6 +767,7 @@ int main()
{
unit_test(test_prepend_scalar_idempotent),
unit_test(test_length),
unit_test(test_equality),
unit_test(test_copy),
unit_test(test_rval_to_scalar),
unit_test(test_rval_to_scalar2),
Expand Down

0 comments on commit d5a7372

Please sign in to comment.