Re: [PATCH 2/4] topology: Change variable type to fix gcc warning

[Takashi Iwai <tiwai@suse.de>]

On Wed, 18 Nov 2015 08:28:09 +0100,
Lin, Mengdong wrote:
> 
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de]
> > Sent: Wednesday, November 18, 2015 3:16 PM
> > To: mengdong.lin@linux.intel.com
> 
> > On Wed, 18 Nov 2015 08:23:07 +0100,
> > mengdong.lin@linux.intel.com wrote:
> > >
> > > From: Mengdong Lin <mengdong.lin@linux.intel.com>
> > >
> > > Fix warning: comparison between signed and unsigned integer
> > > expressions [-Wsign-compare]
> > >
> > > ABI objects use type _le32, which is converted to host unsigned integer.
> > > So the iterator 'i' in a loop as below should also be unsigned.
> > > for (i = 0; i < pcm->num_streams; i++)
> > >                 ^
> > 
> > Using an unsigned int for the generic loop count like i is strange.
> 
> Yes.
> 
> > Rather compare with the original value in the template, which is actually int.
> 
> Are you suggesting not change the code?

No.

> The "pcm->num_streams" here is __le32 defined in ABI:
> struct snd_soc_tplg_pcm {
> 	...
> 	__le32 num_streams;	/* number of streams */
> 	...
> } __attribute__((packed));
> 
> It seems gcc takes it as unsigned integer and so I get the warning.

The problem isn't only about the endianess.
Conceptually, it's wrong to refer directly to le32 data, as it's not
being CPU endian.  It should have always an endian conversion.  Of
course, we support only LE32, so it's no big issue, so far.

What you need to refer to is the value in the template instead.  It's
guaranteed to be CPU endianess, and it's even int.
So, the code would look like:

	for (i = 0; i < pcm_tpl->num_streams; i++)


BTW, I haven't checked whether topology API would "work" on big-endian
architecture; does it return an error properly? 


Takashi


> 
> Thanks
> Mengdong
> 
> > 
> > 
> > Takashi
> > 
> > > Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>
> > >
> > > diff --git a/src/topology/ctl.c b/src/topology/ctl.c index
> > > 7d8787f..6dc3b3d 100644
> > > --- a/src/topology/ctl.c
> > > +++ b/src/topology/ctl.c
> > > @@ -676,7 +676,8 @@ int tplg_add_mixer(snd_tplg_t *tplg, struct
> > snd_tplg_mixer_template *mixer,
> > >  	struct snd_soc_tplg_private *priv = mixer->priv;
> > >  	struct snd_soc_tplg_mixer_control *mc;
> > >  	struct tplg_elem *elem;
> > > -	int ret, i;
> > > +	int ret;
> > > +	unsigned int i;
> > >
> > >  	tplg_dbg(" Control Mixer: %s\n", mixer->hdr.name);
> > >
> > > @@ -743,7 +744,8 @@ int tplg_add_enum(snd_tplg_t *tplg, struct
> > > snd_tplg_enum_template *enum_ctl,  {
> > >  	struct snd_soc_tplg_enum_control *ec;
> > >  	struct tplg_elem *elem;
> > > -	int ret, i;
> > > +	int ret;
> > > +	unsigned int i;
> > >
> > >  	tplg_dbg(" Control Enum: %s\n", enum_ctl->hdr.name);
> > >
> > > diff --git a/src/topology/pcm.c b/src/topology/pcm.c index
> > > 9b7e402..4b7c058 100644
> > > --- a/src/topology/pcm.c
> > > +++ b/src/topology/pcm.c
> > > @@ -522,7 +522,7 @@ int tplg_add_pcm_object(snd_tplg_t *tplg,
> > snd_tplg_obj_template_t *t)
> > >  	struct snd_tplg_pcm_template *pcm_tpl = t->pcm;
> > >  	struct snd_soc_tplg_pcm *pcm;
> > >  	struct tplg_elem *elem;
> > > -	int i;
> > > +	unsigned int i;
> > >
> > >  	tplg_dbg("PCM: %s, DAI %s\n", pcm_tpl->pcm_name,
> > pcm_tpl->dai_name);
> > >
> > > @@ -564,7 +564,7 @@ int tplg_add_link_object(snd_tplg_t *tplg,
> > snd_tplg_obj_template_t *t)
> > >  	struct snd_tplg_link_template *link = t->link;
> > >  	struct snd_soc_tplg_link_config *lk;
> > >  	struct tplg_elem *elem;
> > > -	int i;
> > > +	unsigned int i;
> > >
> > >  	if (t->type != SND_TPLG_TYPE_BE && t->type != SND_TPLG_TYPE_CC)
> > >  		return -EINVAL;
> > > --
> > > 2.5.0
> > >
>