Re: [PATCH QMI LTE v2 08/13] qmi: don't leak cbd and rely on destroy function

[Denis Kenzior <denkenz@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1427 bytes --]

Hi Jonas,

On 04/14/2017 04:36 PM, Jonas Bonn wrote:
> ---
>  drivers/qmimodem/gprs.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/qmimodem/gprs.c b/drivers/qmimodem/gprs.c
> index 05ad4bd..404c975 100644
> --- a/drivers/qmimodem/gprs.c
> +++ b/drivers/qmimodem/gprs.c
> @@ -94,7 +94,7 @@ static void qmi_set_attached(struct ofono_gprs *gprs, int attached,
>  					ofono_gprs_cb_t cb, void *user_data)
>  {
>  	struct gprs_data *data = ofono_gprs_get_data(gprs);
> -	struct cb_data *cbd = cb_data_new(cb, user_data);
> +	struct cb_data *cbd;
>  	struct qmi_param *param;
>  	uint8_t action;
>
> @@ -109,6 +109,7 @@ static void qmi_set_attached(struct ofono_gprs *gprs, int attached,
>  	if (!param)
>  		goto error;
>
> +	cbd = cb_data_new(cb, user_data);
>  	if (qmi_service_send(data->nas, QMI_NAS_ATTACH_DETACH, param,
>  					attach_detach_cb, cbd, g_free) > 0)
>  		return;

So cbd is not freed if qmi_service_send fails.  So you are introducing a 
leak on the fail path.

> @@ -117,8 +118,6 @@ static void qmi_set_attached(struct ofono_gprs *gprs, int attached,
>
>  error:
>  	CALLBACK_WITH_FAILURE(cb, cbd->data);
> -
> -	g_free(cbd);
>  }
>
>  static void get_ss_info_cb(struct qmi_result *result, void *user_data)
>

This entire patch seems unnecessary?  The code looks fine as is.  Or am 
I missing something?

Regards,
-Denis