[alsa-devel] [PATCH] Uninitialized id returned by saffirepro_both_clk_src_get.

[Takashi Iwai]

At Mon, 27 Oct 2014 21:55:48 +0900,
Takashi Sakamoto wrote:
> 
> Iwai-san,
> 
> On Oct 25 2014 20:40, Christian Vogel wrote:
> > snd_bebob_stream_check_internal_clock() may get an id from
> > saffirepro_both_clk_src_get (via clk_src->get()) that was uninitialized.
> > 
> > a) make logic in saffirepro_both_clk_src_get explicit
> > b) test if id used in snd_bebob_stream_check_internal_clock matches array size
> > 
> > Signed-off-by: Takashi Sakamoto <o-takashi at sakamocchi.jp>
> > Signed-off-by: Christian Vogel <vogelchr at vogel.cx>
> > ---
> >  sound/firewire/bebob/bebob_focusrite.c | 62 ++++++++++++++++++++++++++--------
> >  sound/firewire/bebob/bebob_stream.c    | 18 ++++++++--
> >  2 files changed, 63 insertions(+), 17 deletions(-)
> 
> [alsa-devel] [PATCH] Uninitialized id returned by
> saffirepro_both_clk_src_get.
> http://mailman.alsa-project.org/pipermail/alsa-devel/2014-October/082810.html
> 
> Would you please apply this patch to 'for-linus' branch and C.C.ed to
> Linux stable branch, as a bug fix?

OK.  But we need a fix in addition.  See below.

> The subject should be with a prefix 'ALSA: bebob: ', and I hope to add
> 'Reviewed-by' instead of 'Signed-off-by' for myself.

If the patch doesn't come from you, signed-off-by is actually a wrong
tag.  I'll replace it accordingly.

About the patch:
> > +/* Value maps between registers and labels for SaffirePro 10/26. */
> > +static const char saffirepro_clk_maps[][SAFFIREPRO_CLOCK_SOURCE_COUNT] = {
> > +	/* SaffirePro 10 */
> > +	[0] = {
> > +		[SAFFIREPRO_CLOCK_SOURCE_INTERNAL]  =  0,
> > +		[SAFFIREPRO_CLOCK_SOURCE_SKIP]      = -1, /* not supported */

If you need to handle a negative value, you must specify "signed
char".  Without "signed" prefix, it might be unsigned, depending on
the architecture, in the case of char.

> >  static int
> >  saffirepro_both_clk_src_get(struct snd_bebob *bebob, unsigned int *id)
> >  {
> >  	int err;
> > -	u32 value;
> > +	u32 value;       /* clock source read from hw register */
> > +	const char *map;

Ditto.


thanks,

Takashi