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

[linuxtestproject.agent@gmail.com]

Hi Jinseok,

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

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

The original shell test only checked current_governor_ro and
current_driver for readability. The new test also covers
available_governors, current_governor (read), and adds a write
rejection check against current_governor. That is an extension,
not a preservation of semantics. Please update the commit message
body to reflect what is actually added.

> +static void verify_cpuidle(unsigned int i)
> +{
> +	int ret;
> +	char path[PATH_MAX];
> +	char buf[32];
> +
> +	snprintf(path, sizeof(path), "%s/%s", CPUIDLE_PATH, tcases[i].name);
> +
> +	if (tcases[i].is_writable)
> +		fd = open(path, O_RDWR);
> +	else
> +		fd = open(path, O_RDONLY);

open() is a tested subject here -- its return code drives the
TCONF/TFAIL decision. Bare syscalls that are the subject of the
test must be wrapped in TEST(), not called directly. The same
applies to read() and write() below.

> +	ret = read(fd, buf, sizeof(buf));
> +
> +	if (ret < 0)
> +		tst_res(TFAIL | TERRNO, "%s read() failed", path);
> +	else if (ret == 0)
> +		tst_res(TFAIL, "%s read() returned 0", path);
> +	else
> +		tst_res(TPASS, "%s read() successfully", path);

read() is the primary subject of this test and must be wrapped in
TEST() so that TST_RET and TST_ERR are populated.

> +	if (tcases[i].is_writable) {
> +		if (write(fd, "invalid_governor", 16) < 0) {

Same for write().

> +static struct tst_test test = {
> +	.cleanup = cleanup,
> +	.needs_root = 1,

The doc comment only mentions readability. It does not explain why
root is required. Ground Rule 4 requires the reason to appear in
the /*\ ... */ block. Root is needed here for writing to
current_governor and for the save_restore of that path; please
document that.

> + * Verify that selected cpuidle sysfs files are readable.

The test also verifies that writing an invalid governor name to
current_governor is rejected with EINVAL. The description should
cover both aspects.

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