[3/6] topology: Use generic pointer to realloc buffer for private data

Commit Message

[e-mail] April 28, 2016, 8:41 a.m. UTC

From: Mengdong Lin <[e-mail]>

Many element types have private data. So use the generic obj pointer
instead of the type-specific pointer when reallocating the object to
accommodate the private data.

Empty private data will be overlooked.

Signed-off-by: Mengdong Lin <[e-mail]>

Comments

Jaroslav Kysela April 28, 2016, 8:48 a.m. UTC | #1

Dne 28.4.2016 v 10:41 [e-mail] napsal(a):
>  	priv_data_size = ref->data->size;
> +	elem->obj = realloc(elem->obj,
> +			elem->size + priv_data_size);
> +	if (!elem->obj)
> +		return -ENOMEM;

This causes a memory leak when realloc fails. You should free the
original pointer when realloc() fails.

					Jaroslav

Takashi Iwai April 28, 2016, 1:55 p.m. UTC | #2

On Thu, 28 Apr 2016 10:48:36 +0200,
Jaroslav Kysela wrote:
> 
> Dne 28.4.2016 v 10:41 [e-mail] napsal(a):
> >  	priv_data_size = ref->data->size;
> > +	elem->obj = realloc(elem->obj,
> > +			elem->size + priv_data_size);
> > +	if (!elem->obj)
> > +		return -ENOMEM;
> 
> This causes a memory leak when realloc fails. You should free the
> original pointer when realloc() fails.

Right, and the bug (the leak) has been already present before the
patch...


thanks,

Takashi

Lin, Mengdong April 28, 2016, 2:30 p.m. UTC | #3

> -----Original Message-----
> From: Takashi Iwai [mailto:[e-mail]]
> Sent: Thursday, April 28, 2016 9:55 PM
> To: Jaroslav Kysela
> Cc: [e-mail]; [e-mail];
> [e-mail]; Girdwood, Liam R; Lin, Mengdong; Shah, Hardik T; Singh,
> Guneshwor O
> Subject: Re: [PATCH 3/6] topology: Use generic pointer to realloc buffer for
> private data
> 
> On Thu, 28 Apr 2016 10:48:36 +0200,
> Jaroslav Kysela wrote:
> >
> > Dne 28.4.2016 v 10:41 [e-mail] napsal(a):
> > >  	priv_data_size = ref->data->size;
> > > +	elem->obj = realloc(elem->obj,
> > > +			elem->size + priv_data_size);
> > > +	if (!elem->obj)
> > > +		return -ENOMEM;
> >
> > This causes a memory leak when realloc fails. You should free the
> > original pointer when realloc() fails.
> 
> Right, and the bug (the leak) has been already present before the patch...
> 

Okay, we'll fix this. Thanks for pointing out this.

Regards
Mengdong

[e-mail] May 4, 2016, 1:29 a.m. UTC | #4

On 04/28/2016 10:30 PM, Lin, Mengdong wrote:
>> -----Original Message-----
>> From: Takashi Iwai [mailto:[e-mail]]
>> Sent: Thursday, April 28, 2016 9:55 PM
>> To: Jaroslav Kysela
>> Cc: [e-mail]; [e-mail];
>> [e-mail]; Girdwood, Liam R; Lin, Mengdong; Shah, Hardik T; Singh,
>> Guneshwor O
>> Subject: Re: [PATCH 3/6] topology: Use generic pointer to realloc buffer for
>> private data
>>
>> On Thu, 28 Apr 2016 10:48:36 +0200,
>> Jaroslav Kysela wrote:
>>>
>>> Dne 28.4.2016 v 10:41 [e-mail] napsal(a):
>>>>   	priv_data_size = ref->data->size;
>>>> +	elem->obj = realloc(elem->obj,
>>>> +			elem->size + priv_data_size);
>>>> +	if (!elem->obj)
>>>> +		return -ENOMEM;
>>>
>>> This causes a memory leak when realloc fails. You should free the
>>> original pointer when realloc() fails.
>>
>> Right, and the bug (the leak) has been already present before the patch...
>>
>
> Okay, we'll fix this. Thanks for pointing out this.
>

I fixed this issue in v2 series. Would you please have a review?


Thanks again
Mengdong

Takashi Iwai May 8, 2016, 9:52 a.m. UTC | #5

On Wed, 04 May 2016 03:29:33 +0200,
Mengdong Lin wrote:
> 
> 
> 
> On 04/28/2016 10:30 PM, Lin, Mengdong wrote:
> >> -----Original Message-----
> >> From: Takashi Iwai [mailto:[e-mail]]
> >> Sent: Thursday, April 28, 2016 9:55 PM
> >> To: Jaroslav Kysela
> >> Cc: [e-mail]; [e-mail];
> >> [e-mail]; Girdwood, Liam R; Lin, Mengdong; Shah, Hardik T; Singh,
> >> Guneshwor O
> >> Subject: Re: [PATCH 3/6] topology: Use generic pointer to realloc buffer for
> >> private data
> >>
> >> On Thu, 28 Apr 2016 10:48:36 +0200,
> >> Jaroslav Kysela wrote:
> >>>
> >>> Dne 28.4.2016 v 10:41 [e-mail] napsal(a):
> >>>>   	priv_data_size = ref->data->size;
> >>>> +	elem->obj = realloc(elem->obj,
> >>>> +			elem->size + priv_data_size);
> >>>> +	if (!elem->obj)
> >>>> +		return -ENOMEM;
> >>>
> >>> This causes a memory leak when realloc fails. You should free the
> >>> original pointer when realloc() fails.
> >>
> >> Right, and the bug (the leak) has been already present before the patch...
> >>
> >
> > Okay, we'll fix this. Thanks for pointing out this.
> >
> 
> I fixed this issue in v2 series. Would you please have a review?

Now I'm back to online.  Will start reviewing the series in tomorrow
or so.


thanks,

Takashi

Patch

diff --git a/src/topology/data.c b/src/topology/data.c
index 19c31bf..6606ca9 100644
--- a/src/topology/data.c
+++ b/src/topology/data.c
@@ -827,39 +827,29 @@  int tplg_copy_data(struct tplg_elem *elem, struct tplg_elem *ref)
 		return -EINVAL;
 
 	tplg_dbg("Data '%s' used by '%s'\n", ref->id, elem->id);
+	if (!ref->data || !ref->data->size) /* overlook empty private data */
+		return 0;
+
 	priv_data_size = ref->data->size;
+	elem->obj = realloc(elem->obj,
+			elem->size + priv_data_size);
+	if (!elem->obj)
+		return -ENOMEM;
 
 	switch (elem->type) {
 	case SND_TPLG_TYPE_MIXER:
-		elem->mixer_ctrl = realloc(elem->mixer_ctrl,
-			elem->size + priv_data_size);
-		if (!elem->mixer_ctrl)
-			return -ENOMEM;
 		priv = &elem->mixer_ctrl->priv;
 		break;
 
 	case SND_TPLG_TYPE_ENUM:
-		elem->enum_ctrl = realloc(elem->enum_ctrl,
-			elem->size + priv_data_size);
-		if (!elem->enum_ctrl)
-			return -ENOMEM;
 		priv = &elem->enum_ctrl->priv;
 		break;
 
 	case SND_TPLG_TYPE_BYTES:
-		elem->bytes_ext = realloc(elem->bytes_ext,
-			elem->size + priv_data_size);
-		if (!elem->bytes_ext)
-			return -ENOMEM;
 		priv = &elem->bytes_ext->priv;
 		break;
 
-
 	case SND_TPLG_TYPE_DAPM_WIDGET:
-		elem->widget = realloc(elem->widget,
-			elem->size + priv_data_size);
-		if (!elem->widget)
-			return -ENOMEM;
 		priv = &elem->widget->priv;
 		break;