26.12.2021 18:35, Hector Martin пишет:
> +static void brcm_free_alt_fw_paths(const char **alt_paths)
> +{
> + int i;
> +
> + if (!alt_paths)
> + return;
> +
> + for (i = 0; alt_paths[i]; i++)
> + kfree(alt_paths[i]);
> +
> + kfree(alt_paths);
> }
>
> static int brcmf_fw_request_firmware(const struct firmware **fw,
> struct brcmf_fw *fwctx)
> {
> struct brcmf_fw_item *cur = &fwctx->req->items[fwctx->curpos];
> - int ret;
> + int ret, i;
>
> /* Files can be board-specific, first try a board-specific path */
> if (cur->type == BRCMF_FW_TYPE_NVRAM && fwctx->req->board_type) {
> - char *alt_path;
> + const char **alt_paths = brcm_alt_fw_paths(cur->path, fwctx);
>
> - alt_path = brcm_alt_fw_path(cur->path, fwctx->req->board_type);
> - if (!alt_path)
> + if (!alt_paths)
> goto fallback;
>
> - ret = request_firmware(fw, alt_path, fwctx->dev);
> - kfree(alt_path);
> - if (ret == 0)
> - return ret;
> + for (i = 0; alt_paths[i]; i++) {
> + ret = firmware_request_nowarn(fw, alt_paths[i], fwctx->dev);
> + if (ret == 0) {
> + brcm_free_alt_fw_paths(alt_paths);
> + return ret;
> + }
> + }
> + brcm_free_alt_fw_paths(alt_paths);
> }
>
> fallback:
> @@ -641,6 +663,9 @@ static void brcmf_fw_request_done(const struct firmware *fw, void *ctx)
> struct brcmf_fw *fwctx = ctx;
> int ret;
>
> + brcm_free_alt_fw_paths(fwctx->alt_paths);
> + fwctx->alt_paths = NULL;
It looks suspicious that fwctx->alt_paths isn't zero'ed by other code
paths. The brcm_free_alt_fw_paths() should take fwctx for the argument
and fwctx->alt_paths should be set to NULL there.
On the other hand, I'd change the **alt_paths to a fixed-size array.
This should simplify the code, making it easier to follow and maintain.
- const char **alt_paths;
+ char *alt_paths[BRCM_MAX_ALT_FW_PATHS];
Then you also won't need to NULL-terminate the array, which is a common
source of bugs in kernel.
