[alsa-devel] [PATCH] topology: Fix the missing referenced elem ptr when merging private data

Lin, Mengdong mengdong.lin at intel.com
Fri Jul 22 03:48:41 CEST 2016


> -----Original Message-----
> From: Takashi Sakamoto [mailto:o-takashi at sakamocchi.jp]
> Sent: Friday, July 22, 2016 5:16 AM

> > It's better to split to two patches for the different aims.
> > Accumulating patches to log history is preferable, as long as you have
> > concrete wills to improve codes, I believe.

Okay. I split them in v2.

> >
> >> diff --git a/src/topology/data.c b/src/topology/data.c index
> >> 768fc27..397f6a5 100644
> >> --- a/src/topology/data.c
> >> +++ b/src/topology/data.c
> >> @@ -1050,6 +1050,7 @@ int tplg_copy_data(snd_tplg_t *tplg, struct
> tplg_elem *elem,
> >>  		" element '%s'\n", ref->id, elem->id);
> >>  		return -EINVAL;
> >>  	}
> >> +	ref->elem = ref_elem;
> >>
> >>  	tplg_dbg("Data '%s' used by '%s'\n", ref->id, elem->id);
> >>  	/* overlook empty private data */
> >
> > I wonder why the assignment is done in this line. This function still
> > includes some codes to return errors. In this case, it's better to
> > code the assignment in last part of the function.
> >
> > This is a public API. It's better not to touch application's objects
> > in invalid cases.
> 
> Oops. I overlooked. It's in src/topology/tplg_local.h and not a public API...
> Anyway, it's better to consider about the error cases for consistency of object
> state.

Yes. It's better to move to the end of this function, only on success.
Also fixed this in v2 series "topology: Fix for handling widgets' references"

Thanks for your review!
Mengdong



More information about the Alsa-devel mailing list