"Kershner, David A" <[email protected]> writes:
>> -----Original Message-----
>> From: Dan Carpenter [mailto:[email protected]]
>> Sent: Wednesday, July 15, 2015 8:16 AM
>> To: Romer, Benjamin M
>> Cc: [email protected]; [email protected];
>> [email protected]; *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
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
