Re: [LTP] power_management: rewrite runpwtests04.sh in C

Hi Jinseok,

On Wed, Jul 29, 2026, Jinseok Kim wrote:
> power_management: rewrite runpwtests04.sh in C

> The new implementation preserves the original test semantics while
> removing shell dependencies.

The C version does more than the shell test: it also reads
available_governors and current_governor and adds an invalid-governor
write-rejection check, whereas runpwtests04.sh only read
current_governor_ro and current_driver. Could the commit message be
reworded to reflect the added coverage rather than "preserves the
original test semantics"?

> +++ b/testcases/kernel/power_management/.gitignore
> @@ -1 +1,2 @@
>  high_freq_hwp_cap_cppc
> +cpuidle01

Should the entries stay sorted? Appending cpuidle01 after
high_freq_hwp_cap_cppc leaves the file out of alphabetical order
(c before h). Leaf-directory entries are also normally anchored with a
leading '/' (e.g. /cpuidle01), though the existing entry is unanchored
too.

> +static struct tst_test test = {
> +	.test_all = run,
> +	.needs_root = 1,
> +	.save_restore = (const struct tst_path_val[]) {
> +		{ CPUIDLE_PATH "/current_governor", NULL, TST_SR_TCONF },
> +		{}
> +	},
> +};

TST_SR_TCONF expands to TST_SR_TCONF_MISSING | TST_SR_TCONF_RO, so
tst_sys_conf_save() ends the whole test with TCONF at setup when
current_governor is missing or read-only. On a system that exposes only
current_governor_ro, the readable checks for current_governor_ro and
current_driver (which the shell test still ran) would be skipped
entirely. Is gating the full test on current_governor intended, or would
TST_SR_SKIP / a per-file check keep the other checks running?

Verdict - Needs revision

---
Note:

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp