[U-Boot] [PATCH v3 3/3] common: Generic loader for file system

[Anatolij Gustschin <agust@denx.de>]

Hi,

please see some comments below.

On Mon, 25 Jun 2018 21:28:58 +0800
tien.fong.chee at intel.com tien.fong.chee at intel.com wrote:

> +/**
> + * _request_firmware_prepare - Prepare firmware struct.
> + *
> + * @firmware_p: Pointer to pointer to firmware image.
> + * @name: Name of firmware file.
> + * @dbuf: Address of buffer to load firmware into.
> + * @size: Size of buffer.
> + * @offset: Offset of a file for start reading into buffer.
> + *
> + * Return: Negative value if fail, 0 for successful.
> + */
> +static int _request_firmware_prepare(struct firmware **firmware_p,
> +				    const char *name, void *dbuf,
> +				    size_t size, u32 offset)
> +{
> +	struct firmware *firmware;
> +	struct firmware_priv *fw_priv;
> +
> +	*firmware_p = NULL;
> +
> +	if (!name || name[0] == '\0')
> +		return -EINVAL;
> +
> +	firmware = calloc(1, sizeof(*firmware));
> +	if (!firmware)
> +		return -ENOMEM;
> +
> +	fw_priv = calloc(1, sizeof(*fw_priv));
> +	if (!fw_priv) {
> +		free(firmware);
> +		return -ENOMEM;
> +	}

please add a note to API description that the API user should
free() *firmware_p and *firmware_p->priv structs after usage of
request_firmware_into_buf(), otherwise it will always leak memory
while subsequent calls of request_firmware_into_buf() with the
same *firmware_p argument.

Or probably we should better allow request_firmware_into_buf() to
be called multiple times with prepared *firmware_p stuct for
reloading the same firmware image when needed.
Then request_firmware_into_buf() should check if the given
*firmware_p stuct is already initialized and must not call
_request_firmware_prepare() in this case.

> +
> +	fw_priv->name = name;
> +	fw_priv->offset = offset;
> +	firmware->data = dbuf;
> +	firmware->size = size;
> +	firmware->priv = fw_priv;
> +	*firmware_p = firmware;
> +
> +	return 0;
> +}

--
Anatolij