"Kershner, David A" <David.Kershner@xxxxxxxxxx> writes: >> -----Original Message----- >> From: Dan Carpenter [mailto:dan.carpenter@xxxxxxxxxx] >> Sent: Wednesday, July 15, 2015 8:16 AM >> To: Romer, Benjamin M >> Cc: gregkh@xxxxxxxxxxxxxxxxxxx; driverdev-devel@xxxxxxxxxxxxxxxxxxxxxx; >> Jes.Sorensen@xxxxxxxxxx; *S-Par-Maintainer >> Subject: Re: [PATCH] staging: unisys: Add s-Par visorhba >> >> Since you are redoing this anyway... >> >> On Mon, Jul 13, 2015 at 02:38:23PM -0400, Benjamin Romer wrote: >> > + for (vdisk = &devdata->head; vdisk->next; vdisk = vdisk->next) { >> > + if ((scsidev->channel == vdisk->channel) && >> > + (scsidev->id == vdisk->id) && >> > + (scsidev->lun == vdisk->lun)) { >> > + if (atomic_read(&vdisk->error_count) < >> > + VISORHBA_ERROR_COUNT) >> > + atomic_inc(&vdisk->error_count); >> > + else >> > + atomic_set(&vdisk->ios_threshold, >> > + IOS_ERROR_THRESHOLD); >> > + } >> > + } >> >> >> We do this loop all the time, and we're hitting the 80 character >> limit. Make it a define. >> >> #define for_each_vdisk_match(iter, list, match) \ >> for (iter = &list->head; iter->next; iter = iter->next) \ >> if (iter->channel == match->channel && \ >> iter->id == match->id && \ >> iter->lun == match->lun) >> >> Btw, avoid using too many parenthesis. It makes the code harder to read >> and it silences GCC's check for == vs = typos so it can lead to bugs. >> >> Now the loop looks like: >> >> for_each_vdisk_match(vdisk, devdata, scsidev) { >> if (atomic_read(&vdisk->error_count) < >> VISORHBA_ERROR_COUNT) >> atomic_inc(&vdisk->error_count); >> else >> atomic_set(&vdisk->ios_threshold, >> IOS_ERROR_THRESHOLD); >> >> } >> >> (Written in email client. Caveat emptor.) > > Thanks for the comments and I really like this idea. When I put it in the code, > I get the following from checkpatch: > > ERROR: Macros with complex values should be enclosed in parentheses > #31: FILE: drivers/staging/unisys/visorhba/visorhba_main.c:157: > +#define for_each_vdisk_match(iter, list, match) \ > + for (iter = &list->head; iter->next; iter = iter->next) \ > + if (iter->channel == match->channel && \ > + iter->id == match->id && \ > + iter->lun == match->lun) > > total: 1 errors, 0 warnings, 0 checks, 275 lines checked > > Your patch has style problems, please review. > > Any ideas what needs to be wrapped to resolved the checkpatch error? Maybe this: #define for_each_vdisk_match(iter, list, match) \ for (iter = &(list)->head; iter->next; iter = iter->next) \ if (iter->channel == match->channel && \ iter->id == match->id && \ iter->lun == match->lun) My guess it's the &list->head that triggers the issue. Jes _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
