| Message ID | 1397028108-10976-1-git-send-email-k.kozlowski@samsung.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
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
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.
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
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.
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
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;
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(-)