Merge patch series "cmd/setexpr: support concatenation of direct strings"

Heinrich Schuchardt <heinrich.schuchardt@canonical.com> says:

The setexpr.s command allows to concatenate two strings.

According to the description in doc/usage/cmd/setexpr.rst the parameters
value1 and value2 can be either direct values or pointers to a
memory location holding the values.

Unfortunately `setexpr.s <value1> + <value2>` fails if any of the values
is a direct value. $? is set to false.

* Add support for direct values in setexpr.s.
* Correct the unit test for "setexpr.s fred 0".
* Add a new unit test for "setexpr.s fred '1' + '3'" giving '13'.
* Remove invalid memory leak tests

Link: https://lore.kernel.org/r/20250203151029.60265-1-heinrich.schuchardt@canonical.com
This commit is contained in:
Tom Rini
2025-02-07 13:35:32 -06:00
2 changed files with 45 additions and 55 deletions

View File

@@ -35,9 +35,37 @@ struct expr_arg {
};
};
/**
* arg_set_str() - copy string to expression argument
*
* The string is truncated to 64 KiB plus NUL terminator.
*
* @p: pointer to string
* @argp: pointer to expression argument
* Return: 0 on success, -ENOMEM if out of memory
*/
static int arg_set_str(void *p, struct expr_arg *argp)
{
int len;
char *str;
/* Maximum string length of 64 KiB plus NUL terminator */
len = strnlen((char *)p, SZ_64K) + 1;
str = malloc(len);
if (!str) {
printf("Out of memory\n");
return -ENOMEM;
}
memcpy(str, p, len);
str[len - 1] = '\0';
argp->sval = str;
return 0;
}
static int get_arg(char *s, int w, struct expr_arg *argp)
{
struct expr_arg arg;
int ret;
/*
* If the parameter starts with a '*' then assume it is a pointer to
@@ -47,8 +75,6 @@ static int get_arg(char *s, int w, struct expr_arg *argp)
ulong *p;
ulong addr;
ulong val;
int len;
char *str;
addr = hextoul(&s[1], NULL);
switch (w) {
@@ -66,18 +92,10 @@ static int get_arg(char *s, int w, struct expr_arg *argp)
break;
case CMD_DATA_SIZE_STR:
p = map_sysmem(addr, SZ_64K);
/* Maximum string length of 64KB plus terminator */
len = strnlen((char *)p, SZ_64K) + 1;
str = malloc(len);
if (!str) {
printf("Out of memory\n");
return -ENOMEM;
}
memcpy(str, p, len);
str[len - 1] = '\0';
ret = arg_set_str(p, &arg);
unmap_sysmem(p);
arg.sval = str;
if (ret)
return ret;
break;
case 4:
p = map_sysmem(addr, sizeof(u32));
@@ -93,10 +111,14 @@ static int get_arg(char *s, int w, struct expr_arg *argp)
break;
}
} else {
if (w == CMD_DATA_SIZE_STR)
return -EINVAL;
if (w == CMD_DATA_SIZE_STR) {
ret = arg_set_str(s, &arg);
if (ret)
return ret;
} else {
arg.ival = hextoul(s, NULL);
}
}
*argp = arg;
return 0;

View File

@@ -297,31 +297,19 @@ SETEXPR_TEST(setexpr_test_backref, UTF_CONSOLE);
/* Test 'setexpr' command with setting strings */
static int setexpr_test_str(struct unit_test_state *uts)
{
ulong start_mem;
char *buf;
buf = map_sysmem(0, BUF_SIZE);
memset(buf, '\xff', BUF_SIZE);
/*
* Set 'fred' to the same length as we expect to get below, to avoid a
* new allocation in 'setexpr'. That way we can check for memory leaks.
*/
ut_assertok(env_set("fred", "x"));
start_mem = ut_check_free();
strcpy(buf, "hello");
ut_asserteq(1, run_command("setexpr.s fred 0", 0));
ut_assertok(ut_check_delta(start_mem));
ut_asserteq(0, run_command("setexpr.s fred 0", 0));
ut_asserteq_str("0", env_get("fred"));
strcpy(buf, "hello");
ut_assertok(env_set("fred", "12345"));
start_mem = ut_check_free();
ut_assertok(run_command("setexpr.s fred *0", 0));
ut_asserteq_str("hello", env_get("fred"));
/*
* This fails in CI at present.
*
* ut_assertok(ut_check_delta(start_mem));
*/
unmap_sysmem(buf);
@@ -332,45 +320,25 @@ SETEXPR_TEST(setexpr_test_str, UTF_CONSOLE);
/* Test 'setexpr' command with concatenating strings */
static int setexpr_test_str_oper(struct unit_test_state *uts)
{
ulong start_mem;
char *buf;
/* Test concatenation of strings */
ut_assertok(run_command("setexpr.s fred '1' + '3'", 0));
ut_asserteq_str("13", env_get("fred"));
buf = map_sysmem(0, BUF_SIZE);
memset(buf, '\xff', BUF_SIZE);
strcpy(buf, "hello");
strcpy(buf + 0x10, " there");
start_mem = ut_check_free();
ut_asserteq(1, run_command("setexpr.s fred *0 * *10", 0));
ut_assertok(ut_check_delta(start_mem));
ut_assert_nextline("invalid op");
ut_assert_console_end();
/*
* Set 'fred' to the same length as we expect to get below, to avoid a
* new allocation in 'setexpr'. That way we can check for memory leaks.
*/
ut_assertok(env_set("fred", "12345012345"));
start_mem = ut_check_free();
ut_assertok(run_command("setexpr.s fred *0 + *10", 0));
ut_asserteq_str("hello there", env_get("fred"));
/*
* This check does not work with sandbox_flattree, apparently due to
* memory allocations in env_set().
*
* The truetype console produces lots of memory allocations even though
* the LCD display is not visible. But even without these, it does not
* work.
*
* A better test would be for dlmalloc to record the allocs and frees
* for a particular caller, but that is not supported.
*
* For now, drop this test.
*
* ut_assertok(ut_check_delta(start_mem));
*/
unmap_sysmem(buf);
return 0;