On Fri, 2018-06-29 at 14:13 +0200, Anatolij Gustschin wrote:
Hi,
please see some comments below.
On Mon, 25 Jun 2018 21:28:58 +0800 tien.fong.chee@intel.com tien.fong.chee@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.
Okay, i will add the description into doc. I can create a function to release memory allocation and setting both *firmware_p and fw_priv to null for user.
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.
Okay, this should work in this way by checking both *firmware_p and fw_priv, if they are not null, memory allocation would be skipped for better performance. However, *firmware_p always need to get updated with function arguments, because it could be chunk by chunk transferring if image is larger than available buffer.
- fw_priv->name = name;
- fw_priv->offset = offset;
- firmware->data = dbuf;
- firmware->size = size;
- firmware->priv = fw_priv;
- *firmware_p = firmware;
- return 0;
+}
-- Anatolij