Messages in this thread | | | Date | Wed, 21 Oct 2009 19:07:07 +0200 | From | Tilman Schmidt <> | Subject | Re: checkpatch.pl false positive? "ERROR: do not use assignment in if condition" |
| |
Andy Whitcroft schrieb: > On Tue, Oct 20, 2009 at 08:58:20AM -0700, Randy Dunlap wrote: >> On Tue, 20 Oct 2009 17:50:45 +0200 Tilman Schmidt wrote: >> >>> The command >>> ./scripts/checkpatch.pl -f drivers/isdn/gigaset/bas-gigaset.c >>> produces a lot of "ERROR" messages like these: >>> >>> ERROR: do not use assignment in if condition >>> #608: FILE: isdn/gigaset/bas-gigaset.c:608: >>> + if ((ret = usb_submit_urb(ucs->urb_cmd_in, GFP_ATOMIC)) != 0) { >>> >>> ERROR: do not use assignment in if condition >>> #745: FILE: isdn/gigaset/bas-gigaset.c:745: >>> + if ((ucs->rcvbuf = kmalloc(l, GFP_ATOMIC)) == NULL) { >>> >>> ERROR: do not use assignment in if condition >>> #753: FILE: isdn/gigaset/bas-gigaset.c:753: >>> + if ((rc = atread_submit(cs, BAS_TIMEOUT)) < 0) { >>> >>> As far as I can see there's nothing wrong with these lines. In particular, >>> I cannot find anything in Documentation/CodingStyle that would prohibit an >>> assignment inside an 'if' condition. >> Yes, we don't try to list Every Possible Problem in CodingStyle,
Well, that's not very nice towards small time developers like me. I try to follow CodingStyle as best I can, only to get smacked with a checkpatch ERROR for something that isn't even hinted at there. If a way of coding is such an abomination that it merits an ERROR from checkpatch (meaning " change this or see your submission rejected"), then it also merits being mentioned in CodingStyle.
>> but emails on lkml over the past several years try to discourage such >> assignments inside if's because they can be confusing, difficult to read, >> and generally are not helping to produce any better code output than >> using: >> ret = usb_submit_urb(args); >> if (ret) { >> foo; >> bar; >> blah(); >> }
I don't agree, but judging from your wording you don't want to hear any arguments, so I'll leave it at that. I just want to know *beforehand* what I have to do in order to get my code merged.
> What he said :). We try and codify the preferred style where there is > a preference. That preference was made by reviewers as it is much > harder to accidentally do the following when you meant the assignment or > visa versa:
But does a mere preference by some reviewers warrant an ERROR message, when even a line that exceeds the 80 character limit, which is elaborately justified in CodingStyle, produces only a warning?
Again, I don't want to start a discussion about that preference. I know now that it exists, and I am in no position to question it. But how many of those preferences may yet turn up to bite me? If people are feeling so strongly about a preference that those who do not honor it must be slapped with a checkpatch ERROR then they should also find the energy to add an appropriate warning about that to CodingStyle so that coders have a chance to avoid it. That's only fair.
Thanks, Tilman
-- Tilman Schmidt E-Mail: tilman@imap.cc Bonn, Germany Diese Nachricht besteht zu 100% aus wiederverwerteten Bits. Ungeöffnet mindestens haltbar bis: (siehe Rückseite)
[unhandled content-type:application/pgp-signature] | |