On Wed, 18 Nov 2015 16:14:43 +0100, Lin, Mengdong wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Wednesday, November 18, 2015 4:19 PM
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++)
Okay, I'll revise the patch. Thanks for your suggestion.
BTW, I haven't checked whether topology API would "work" on big-endian architecture; does it return an error properly?
Sorry, no. I'll check if an API can tell us the host endianness and let topology return an error for big-endian machines.
I had thought to use this API when generating the ABI objects from host type to _le32: uint32_t htole32(uint32_t host_32bits);
Yes, that should work. But then, as I pointed out, many current codes directly refer to the converted data, and they are broken.
But since I have no big-endian machines to test this, maybe it's better to return an error for them.
It's the easiest option, of course :)
Takashi