Re: [LTP] fw_load: Modernize ltp_fw_load kernel module

[linuxtestproject.agent@gmail.com]

Hi Andrea,

On Wed, Jun 10 2026, Andrea Cervesato wrote:
> fw_load: Modernize ltp_fw_load kernel module

--- [PATCH 2/3] ---

> +static void cleanup(void)
> +{
> +	struct fw_data *fw;
> +
> +	for (int i = fw_count - 1; i >= 0; i--) {
> +		fw = &firmware[i];
> +
> +		if (access(fw->file, F_OK) != -1)
> +			SAFE_UNLINK(fw->file);
> +
> +		if (access(fw->dir, F_OK) != -1)
> +			remove(fw->dir);
> +	}

The old code tracked whether each directory was created by the test
(the `remove_dir` flag) and only removed directories the test itself
made. The new code unconditionally attempts `remove()` on every
directory in the `firmware[]` array.

If a directory such as `/lib/firmware/updates/` existed before the
test and happened to be empty after the test file is unlinked,
`remove()` would delete it even though the test did not create it.

Could a flag (or an `access()` check before `SAFE_MKDIR`) be added
to restore the previous behaviour?

--- [PATCH 3/3] ---

> + * [Algorithm]
> + *
> + * - Set the firmware search path to the LTP temporary directory
> + * - Create ``FW_NUM - 1`` firmware files there, each named
> + *   ``n<i>_load_tst.fw`` and filled with a known byte pattern
> + * - Add one fake firmware entry that has no file on disk
> + * - Load the helper module with ``fw_size`` matching the blob size

The algorithm says the module is loaded after the firmware files are
created, but the code loads the module before creating the files
(lines 115-119). The order does not affect correctness since
`request_firmware()` is only triggered when writing to `fwnum`, but
the `[Algorithm]` section should match the actual code order.

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