Hi Ilpo,
On 10/18/24 1:46 AM, Ilpo Järvinen wrote:
On Thu, 17 Oct 2024, Reinette Chatre wrote:
@@ -138,15 +139,26 @@ static int mbm_run_test(const struct resctrl_test *test, const struct user_param .setup = mbm_setup, .measure = mbm_measure, };
- char *endptr = NULL;
- size_t span = 0; int ret;
remove(RESULT_FILE_NAME);
- if (uparams->benchmark_cmd[0] && strcmp(uparams->benchmark_cmd[0], "fill_buf") == 0) {
if (uparams->benchmark_cmd[1]) {errno = 0;span = strtoul(uparams->benchmark_cmd[1], &endptr, 10);if (errno || *endptr != '\0')This no longer catches "" string as error. I tested strtoul() with an empty string and errno remains at 0.
return -errno;Another issue is that in cases where errno=0 (both *endptr != '\0' and endptr == uparams->benchmark_cmd[1]), this function doesn't return a proper error code but -0.
Thank you for catching this. I addressed it with the fixup below:
@@ -146,11 +146,11 @@ static int mbm_run_test(const struct resctrl_test *test, const struct user_param remove(RESULT_FILE_NAME);
if (uparams->benchmark_cmd[0] && strcmp(uparams->benchmark_cmd[0], "fill_buf") == 0) { - if (uparams->benchmark_cmd[1]) { + if (uparams->benchmark_cmd[1] && *uparams->benchmark_cmd[1] != '\0') { errno = 0; span = strtoul(uparams->benchmark_cmd[1], &endptr, 10); if (errno || *endptr != '\0') - return -errno; + return -EINVAL; } }
The above does not address all that can go wrong when user, for example, runs: resctrl_tests -t mbm fill_buf "" The above snippet will ignore the invalid value and focus on the issue addressed in this patch, which is to not print DEFAULT_SPAN when that is not the size used.
At this point of the series run_benchmark() still uses strtoul() to parse the empty string. This is fixed in the later patch "selftests/resctrl: Make benchmark parameter passing robust" that will also be fixed to handle the empty string.
Reinette