Misleading messages from match 0.6.1-rc1

Misleading messages from match 0.6.1-rc1

[Toomas Soome]

Hi!

While checking illumos soource, smatch is reporting:

/code/illumos-gate/usr/src/tools/proto/root_sparc-nd/opt/onbld/bin/sparc/smatch: ../../common/fs/ufs/ufs_vnops.c:737 wrip() warn: subtract is higher precedence than shift

The code is:

http://src.illumos.org/source/xref/illumos-gate/usr/src/uts/common/fs/ufs/ufs_vnops.c#737

limit = MIN(UFS_MAXOFFSET_T, limit);

Where MIN is defined:

#define	MIN(a, b)	((a) < (b) ? (a) : (b))

As seen from those facts, we have no subtraction nor shift. In code above, limit is unsigned 64-bit, UFS_MAXOFFSET_T is signed.


rgds,
Toomas

Re: Misleading messages from match 0.6.1-rc1

[Dan Carpenter]

On Wed, Feb 05, 2020 at 08:51:11AM +0200, Toomas Soome wrote:
> 
> Hi!
> 
> While checking illumos soource, smatch is reporting:
> 
> /code/illumos-gate/usr/src/tools/proto/root_sparc-nd/opt/onbld/bin/sparc/smatch: ../../common/fs/ufs/ufs_vnops.c:737 wrip() warn: subtract is higher precedence than shift
> 
> The code is:
> 
> http://src.illumos.org/source/xref/illumos-gate/usr/src/uts/common/fs/ufs/ufs_vnops.c#737
> 
> limit = MIN(UFS_MAXOFFSET_T, limit);
> 
> Where MIN is defined:
> 
> #define	MIN(a, b)	((a) < (b) ? (a) : (b))
> 
> As seen from those facts, we have no subtraction nor shift. In code above, limit is unsigned 64-bit, UFS_MAXOFFSET_T is signed.
> 

It's complaining about UFS_MAXOFFSET_T.

#define	UFS_MAXOFFSET_T	((1LL << NBBY * sizeof (daddr32_t) + DEV_BSHIFT - 1) \
							- 1)

I assume the code is correct but adding parenthese around the
(DEV_BSHIFT - 1) doesn't help explain where the minus one comes from.

regards,
dan carpenter

Re: Misleading messages from match 0.6.1-rc1

[Toomas Soome]

> On 5. Feb 2020, at 09:05, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> 
> On Wed, Feb 05, 2020 at 08:51:11AM +0200, Toomas Soome wrote:
>> 
>> Hi!
>> 
>> While checking illumos soource, smatch is reporting:
>> 
>> /code/illumos-gate/usr/src/tools/proto/root_sparc-nd/opt/onbld/bin/sparc/smatch: ../../common/fs/ufs/ufs_vnops.c:737 wrip() warn: subtract is higher precedence than shift
>> 
>> The code is:
>> 
>> http://src.illumos.org/source/xref/illumos-gate/usr/src/uts/common/fs/ufs/ufs_vnops.c#737
>> 
>> limit = MIN(UFS_MAXOFFSET_T, limit);
>> 
>> Where MIN is defined:
>> 
>> #define	MIN(a, b)	((a) < (b) ? (a) : (b))
>> 
>> As seen from those facts, we have no subtraction nor shift. In code above, limit is unsigned 64-bit, UFS_MAXOFFSET_T is signed.
>> 
> 
> It's complaining about UFS_MAXOFFSET_T.
> 
> #define	UFS_MAXOFFSET_T	((1LL << NBBY * sizeof (daddr32_t) + DEV_BSHIFT - 1) \
> 							- 1)
> 
> I assume the code is correct but adding parenthese around the
> (DEV_BSHIFT - 1) doesn't help explain where the minus one comes from.
> 
> regards,
> dan carpenter

Oh, it is my oversight, totally missed the alternate define. That case is actually (1 << 40) - 1 and indeed, the warning is to tell us that we might use () around the right side of << operator.

Thanks,
Toomas