[LTP] [PATCH v6] high_freq_hwp_cap_cppc.c: new test
Andrea Cervesato
andrea.cervesato@suse.com
Wed Mar 25 13:24:15 CET 2026
Hi Piotr,
Thanks for the new test. A few issues below.
> +/*
> + * Copyright (C) 2026 Intel - http://www.intel.com/
> + */
The copyright line should include an author name and email, e.g.:
Copyright (c) 2026 Piotr Kubaj <piotr.kubaj@intel.com>
> + if (i)
> + SAFE_FILE_SCANF(path, "%d", &online);
SAFE_FILE_SCANF with "%d" expects a pointer to int, but `online` is
bool (typically 1 byte). This is undefined behavior and will likely
corrupt the stack on little-endian. Please use `int online = 1;`
instead.
This should be spotted by any static analyzer or clangd..and even
gcc shows this error.
> + snprintf(path, sizeof(path), "/dev/cpu/%d/msr", i);
> + int fd = SAFE_OPEN(path, O_RDONLY);
> +
> + SAFE_PREAD(1, fd, &msr_highest_perf, sizeof(msr_highest_perf), 0x771);
SAFE_CLOSE(fd) right after it has been used. There's no need to keep it
open after pread().
> + msr_highest_perf &= (1ULL << 8) - 1;
0x771 and the 8-bit mask are magic numbers. Named defines would
help readability, e.g.:
#define MSR_HWP_CAPABILITIES 0x771
#define HIGHEST_PERF_MASK 0xFF
The test fails because msr device is not available, so you need:
.needs_drivers = (const char *const []) {
"msr",
NULL
},
And probably the CONFIG_X86_MSR=y check as well.
Regards,
--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com
More information about the ltp
mailing list