----- Original Message ----- From: oleg.drokin@xxxxxxxxx To: lidza.louina@xxxxxxxxxx Cc: devel@xxxxxxxxxxxxxxxxxxxx, gregkh@xxxxxxxxxxxxxxxxxxx, lustre-devel@xxxxxxxxxxxxxxxx Sent: Thursday, June 23, 2016 12:50:01 PM GMT -08:00 US/Canada Pacific Subject: Re: [lustre-devel] [PATCH v2] staging/lustre/lnet: changes value to correct type for assignment On Jun 23, 2016, at 3:27 PM, Lidza Louina wrote: > > ----- Original Message ----- > From: oleg.drokin@xxxxxxxxx > To: lidza.louina@xxxxxxxxxx > Cc: gregkh@xxxxxxxxxxxxxxxxxxx, lustre-devel@xxxxxxxxxxxxxxxx, devel@xxxxxxxxxxxxxxxxxxxx, andreas.dilger@xxxxxxxxx > Sent: Thursday, June 23, 2016 12:14:28 PM GMT -08:00 US/Canada Pacific > Subject: Re: [lustre-devel] [PATCH v2] staging/lustre/lnet: changes value to correct type for assignment > > > On Jun 23, 2016, at 2:56 PM, Lidza Louina wrote: > >> The code attempted to add an unsigned int to a an unsigned 64-bit >> integer. This patch makes the code use the correct type of int to >> suppress a smatch warning. > > I think you might want to update the commit message too, because > it does not really make much sense now. > > How about > > "The patch changes a value's type so it can be assigned correctly. > This problem was caught by smatch." I think that still does not convey the idea of teh change, also there's no assignment going on here. Perhaps To make the shift safer, have it operate on a 64 bit integer instead of default 32bit? There's no bug yet, so this is just for future-proofing. This potential problem was caught by smatch. Also you'll need to update the patch subject. I didn't want to copy you word-for-word: [PATCH v3] staging/lustre/lnet: changes type for safer shift The shift was potentially unsafe because of conflicting types. This patch changes the regular int (1) to an unsigned long long int because rec->rec_lh_cookie is an unsigned 64-bit value. This could be a problem down the line. This was caught by smatch. > >> >> Signed-off-by: Lidza Louina <lidza.louina@xxxxxxxxxx> >> --- >> drivers/staging/lustre/lnet/lnet/api-ni.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/staging/lustre/lnet/lnet/api-ni.c b/drivers/staging/lustre/lnet/lnet/api-ni.c >> index 346db89..fc5b763 100644 >> --- a/drivers/staging/lustre/lnet/lnet/api-ni.c >> +++ b/drivers/staging/lustre/lnet/lnet/api-ni.c >> @@ -513,7 +513,7 @@ lnet_res_lh_initialize(struct lnet_res_container *rec, lnet_libhandle_t *lh) >> unsigned int hash; >> >> lh->lh_cookie = rec->rec_lh_cookie; >> - rec->rec_lh_cookie += 1 << ibits; >> + rec->rec_lh_cookie += (1ULL << ibits); >> >> hash = (lh->lh_cookie >> ibits) & LNET_LH_HASH_MASK; >> >> -- >> 1.9.1 >> >> >> >> _______________________________________________ >> lustre-devel mailing list >> lustre-devel@xxxxxxxxxxxxxxxx >> http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org > > _______________________________________________ > lustre-devel mailing list > lustre-devel@xxxxxxxxxxxxxxxx > http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
