[PATCH] ASoC: core: Fix Sparse incompatible types warning

[PATCH] ASoC: core: Fix Sparse incompatible types warning

[Krzysztof Kozlowski]

Fix following Sparse warning:
sound/soc/soc-core.c:252:20: error: incompatible types in comparison expression (different type sizes)

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 sound/soc/soc-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 051c006281f5..41f6178249df 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -249,7 +249,7 @@ static ssize_t codec_reg_write_file(struct file *file,
 	struct snd_soc_codec *codec = file->private_data;
 	int ret;
 
-	buf_size = min(count, (sizeof(buf)-1));
+	buf_size = min(count, (size_t)(sizeof(buf)-1));
 	if (copy_from_user(buf, user_buf, buf_size))
 		return -EFAULT;
 	buf[buf_size] = 0;
-- 
1.8.3.2

Re: [PATCH] ASoC: core: Fix Sparse incompatible types warning

[Lars-Peter Clausen]

On 04/09/2014 09:21 AM, Krzysztof Kozlowski wrote:
> Fix following Sparse warning:
> sound/soc/soc-core.c:252:20: error: incompatible types in comparison expression (different type sizes)
>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---
>   sound/soc/soc-core.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index 051c006281f5..41f6178249df 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -249,7 +249,7 @@ static ssize_t codec_reg_write_file(struct file *file,
>   	struct snd_soc_codec *codec = file->private_data;
>   	int ret;
>
> -	buf_size = min(count, (sizeof(buf)-1));
> +	buf_size = min(count, (size_t)(sizeof(buf)-1));

But shouldn't the type of sizeof already be size_t?

Re: [PATCH] ASoC: core: Fix Sparse incompatible types warning

[Krzysztof Kozlowski]

On śro, 2014-04-09 at 12:57 +0200, Lars-Peter Clausen wrote:
> On 04/09/2014 09:21 AM, Krzysztof Kozlowski wrote:
> > Fix following Sparse warning:
> > sound/soc/soc-core.c:252:20: error: incompatible types in comparison expression (different type sizes)
> >
> > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > ---
> >   sound/soc/soc-core.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> > index 051c006281f5..41f6178249df 100644
> > --- a/sound/soc/soc-core.c
> > +++ b/sound/soc/soc-core.c
> > @@ -249,7 +249,7 @@ static ssize_t codec_reg_write_file(struct file *file,
> >   	struct snd_soc_codec *codec = file->private_data;
> >   	int ret;
> >
> > -	buf_size = min(count, (sizeof(buf)-1));
> > +	buf_size = min(count, (size_t)(sizeof(buf)-1));
> 
> But shouldn't the type of sizeof already be size_t?

Hmmm... yes it should. It looks like a false positive from Sparse so the
commit message should be rather "Suppress" than "Fix".

Best regards,
Krzysztof

Re: [PATCH] ASoC: core: Fix Sparse incompatible types warning

[Lars-Peter Clausen]

On 04/09/2014 01:12 PM, Krzysztof Kozlowski wrote:
> On śro, 2014-04-09 at 12:57 +0200, Lars-Peter Clausen wrote:
>> On 04/09/2014 09:21 AM, Krzysztof Kozlowski wrote:
>>> Fix following Sparse warning:
>>> sound/soc/soc-core.c:252:20: error: incompatible types in comparison expression (different type sizes)
>>>
>>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>> ---
>>>    sound/soc/soc-core.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
>>> index 051c006281f5..41f6178249df 100644
>>> --- a/sound/soc/soc-core.c
>>> +++ b/sound/soc/soc-core.c
>>> @@ -249,7 +249,7 @@ static ssize_t codec_reg_write_file(struct file *file,
>>>    	struct snd_soc_codec *codec = file->private_data;
>>>    	int ret;
>>>
>>> -	buf_size = min(count, (sizeof(buf)-1));
>>> +	buf_size = min(count, (size_t)(sizeof(buf)-1));
>>
>> But shouldn't the type of sizeof already be size_t?
>
> Hmmm... yes it should. It looks like a false positive from Sparse so the
> commit message should be rather "Suppress" than "Fix".

I'm pretty sure it is a bug in sparse, we shouldn't suppress those, but 
rather fix them in sparse itself.

Re: [PATCH] ASoC: core: Fix Sparse incompatible types warning

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 791 bytes --]

On Wed, Apr 09, 2014 at 01:30:47PM +0200, Lars-Peter Clausen wrote:
> On 04/09/2014 01:12 PM, Krzysztof Kozlowski wrote:
> >On śro, 2014-04-09 at 12:57 +0200, Lars-Peter Clausen wrote:
> >>On 04/09/2014 09:21 AM, Krzysztof Kozlowski wrote:

> >>>-	buf_size = min(count, (sizeof(buf)-1));
> >>>+	buf_size = min(count, (size_t)(sizeof(buf)-1));

> >>But shouldn't the type of sizeof already be size_t?

> >Hmmm... yes it should. It looks like a false positive from Sparse so the
> >commit message should be rather "Suppress" than "Fix".

> I'm pretty sure it is a bug in sparse, we shouldn't suppress those, but
> rather fix them in sparse itself.

Either that or there's something else going on that hasn't been properly
understood.  The above just looks completely bogus.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] ASoC: core: Fix Sparse incompatible types warning

[Lars-Peter Clausen]

On 04/09/2014 10:25 PM, Mark Brown wrote:
> On Wed, Apr 09, 2014 at 01:30:47PM +0200, Lars-Peter Clausen wrote:
>> On 04/09/2014 01:12 PM, Krzysztof Kozlowski wrote:
>>> On śro, 2014-04-09 at 12:57 +0200, Lars-Peter Clausen wrote:
>>>> On 04/09/2014 09:21 AM, Krzysztof Kozlowski wrote:
>
>>>>> -	buf_size = min(count, (sizeof(buf)-1));
>>>>> +	buf_size = min(count, (size_t)(sizeof(buf)-1));
>
>>>> But shouldn't the type of sizeof already be size_t?
>
>>> Hmmm... yes it should. It looks like a false positive from Sparse so the
>>> commit message should be rather "Suppress" than "Fix".
>
>> I'm pretty sure it is a bug in sparse, we shouldn't suppress those, but
>> rather fix them in sparse itself.
>
> Either that or there's something else going on that hasn't been properly
> understood.  The above just looks completely bogus.
>

I had a look at the sparse code and the problem is that it sets the default 
return type of sizeof according to the type of the host it was compiled on 
(either unsigned int or unsigned long). It can be overwritten by switches like 
-m32, but of course wont work when cross compiling. So if your host system is 
64bit, but your target system is 32bit you'll get that warning.

- Lars

Re: [PATCH] ASoC: core: Fix Sparse incompatible types warning

[Al Viro]

On Wed, Apr 09, 2014 at 10:28:59PM +0200, Lars-Peter Clausen wrote:

> I had a look at the sparse code and the problem is that it sets the
> default return type of sizeof according to the type of the host it
> was compiled on (either unsigned int or unsigned long). It can be
> overwritten by switches like -m32, but of course wont work when
> cross compiling. So if your host system is 64bit, but your target
> system is 32bit you'll get that warning.

Not that simple.  First of all, you *need* to tell sparse what target
to expect; size_t is the least of your troubles - sizeof(long) has
far more widespread impact.  If you don't get -m64 or -m32 in CFLAGS
(and on quite a few cross-builds you get it, simply because the target
is biarch and gcc itself needs to know what to generate), you need
to set it in CHECKFLAGS, along with other target-specific things.
Example:
arch/ia64/Makefile:21:CHECKFLAGS        += -m64 -D__ia64=1 -D__ia64__=1 -D_LP64 -D__LP64__

With the defaults being what they are (since commit 7aa79f "Adding default for
m64/m32 handle"), we probably need explicit -m32 in CHECKFLAGS for a bunch
of 32bit targets.  Defaults are iffy, BTW - it's not even "do as host does",
it's "64bit if the host is amd64, 32bit otherwise" ;-/

Again, CHECKFLAGS need to be set; that's normally done in arch/*/Makefile.

Re: [PATCH] ASoC: core: Fix Sparse incompatible types warning

[Krzysztof Kozlowski]

On czw, 2014-04-10 at 04:01 +0100, Al Viro wrote:
> On Wed, Apr 09, 2014 at 10:28:59PM +0200, Lars-Peter Clausen wrote:
> 
> > I had a look at the sparse code and the problem is that it sets the
> > default return type of sizeof according to the type of the host it
> > was compiled on (either unsigned int or unsigned long). It can be
> > overwritten by switches like -m32, but of course wont work when
> > cross compiling. So if your host system is 64bit, but your target
> > system is 32bit you'll get that warning.
> 
> Not that simple.  First of all, you *need* to tell sparse what target
> to expect; size_t is the least of your troubles - sizeof(long) has
> far more widespread impact.  If you don't get -m64 or -m32 in CFLAGS
> (and on quite a few cross-builds you get it, simply because the target
> is biarch and gcc itself needs to know what to generate), you need
> to set it in CHECKFLAGS, along with other target-specific things.
> Example:
> arch/ia64/Makefile:21:CHECKFLAGS        += -m64 -D__ia64=1 -D__ia64__=1 -D_LP64 -D__LP64__
> 
> With the defaults being what they are (since commit 7aa79f "Adding default for
> m64/m32 handle"), we probably need explicit -m32 in CHECKFLAGS for a bunch
> of 32bit targets.  Defaults are iffy, BTW - it's not even "do as host does",
> it's "64bit if the host is amd64, 32bit otherwise" ;-/
> 
> Again, CHECKFLAGS need to be set; that's normally done in arch/*/Makefile.

In my case it was cross compile on x86_64 for ARM so adding -m32 to
CHECKFLAGS in arch/arm/Makefile helped.

Best regards,
Krzysztof