Hello!
I pulled in the smatch update today (I was at 73de935ee75c55e09be67a995cd9c8a88aea5451 before) and
immediately I got a lot of extra warnings where random integer comparisons were marked as e.g.:
lustre/mdt/mdt_reint.c:668 mdt_reint_setattr() warn: we tested 'ma->ma_attr.la_valid' before and it was 'false'
if ((ma->ma_attr.la_valid & LA_SIZE) ||
...
} else if ((ma->ma_valid & MA_LMV) && (ma->ma_valid & MA_INODE)) {
struct lu_buf *buf = &info->mti_buf;
struct mdt_lock_handle *lh;
668 if (ma->ma_attr.la_valid != 0)
Well, that's not really a boolean, could it be smart enough that when we use bitwise logical operands on the value,
or compare it to stuff other than 0 and 1, the variable becomes "not boolean" and does not generate those warnings
unless we compare with the exact same value (and then that value is printed)?
I guess ability to look at struct members now also enables another class of false positives best seen as:
if (lqe->lqe_qunit == 0) {
/* lqe was read from disk, but neither qunit, nor edquot flag
* were initialized */
qmt_adjust_qunit(env, lqe);
if (lqe->lqe_qunit != 0)
qmt_adjust_edquot(lqe, cfs_time_current_sec());
Which now generates:
lustre/quota/qmt_entry.c:695 qmt_revalidate() warn: we tested 'lqe->lqe_qunit' before and it was 'false'
I guess it has no knowledge that lqe might be modified in that call and I'd just have to live with this one (to add
insult here, the field is not a boolean thing anyway and holds size).
Also a couple of really bizzare warnings that I have no explanation for:
lustre/lfsck/lfsck_layout.c:2388 lfsck_layout_recreate_lovea() warn: variable dereferenced before check 'handle' (see line 2384)
lustre/lfsck/lfsck_layout.c:2413 lfsck_layout_recreate_lovea() warn: variable dereferenced before check 'handle' (see line 2409)
The code here is (full file: http://git.whamcloud.com/fs/lustre-release.git/blob/refs/heads/master:/lustre/lfsck/lfsck_layout.c):
2409 if (bk->lb_param & LPF_DRYRUN)
GOTO(unlock_parent, rc = 1);
dt_write_unlock(env, parent);
2413 if (handle != NULL)
dt_trans_stop(env, dt, handle);
huh?!
(and yes, I run in --spammy mode and just filter out all other false positives that I already know about as I feel there's extra value
in seeing the new ones when they appear).
I have not looked at the commit history in any sort of depth yet to see if anything could be easily done about it,
and mostly write this in the hopes that perhaps this might be a known problem with some solutions already by any chance? ;)
Bye,
Oleg--
To unsubscribe from this list: send the line "unsubscribe smatch" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
