diff --git a/cmd/setexpr.c b/cmd/setexpr.c index e111b8ba98a..c45fa85c887 100644 --- a/cmd/setexpr.c +++ b/cmd/setexpr.c @@ -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,9 +111,13 @@ static int get_arg(char *s, int w, struct expr_arg *argp) break; } } else { - if (w == CMD_DATA_SIZE_STR) - return -EINVAL; - arg.ival = hextoul(s, NULL); + if (w == CMD_DATA_SIZE_STR) { + ret = arg_set_str(s, &arg); + if (ret) + return ret; + } else { + arg.ival = hextoul(s, NULL); + } } *argp = arg; diff --git a/test/cmd/setexpr.c b/test/cmd/setexpr.c index 7f318a42ead..5e9b577fe36 100644 --- a/test/cmd/setexpr.c +++ b/test/cmd/setexpr.c @@ -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;