linux-kernel - Re: checkpatch.pl false positive? "ERROR: do not use assignment in if condition"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 20 Oct 2009 17:24:00 +0100
From:	Andy Whitcroft <apw@...onical.com>
To:	Randy Dunlap <rdunlap@...otime.net>
Cc:	Tilman Schmidt <tilman@...p.cc>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: checkpatch.pl false positive? "ERROR: do not use assignment in
 if condition"

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,
> 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();
> 	}
> 
> > PS: I know the file has other problems. That's why I'm trying to
> > checkpatch it in the first place.

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:

    if ((rc == foo) < 10)

-apw
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/