linux-kernel - Re: [PATCH 1/6] staging: android: binder: Remove some funny && usage

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]

Message-Id: <1245249469.5982.251.camel@desktop>
Date:	Wed, 17 Jun 2009 07:37:49 -0700
From:	Daniel Walker <dwalker@...o99.com>
To:	Jeremy Fitzhardinge <jeremy@...p.org>
Cc:	Greg Kroah-Hartman <greg@...ah.com>,
	Brian Swetland <swetland@...gle.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/6] staging: android: binder: Remove some funny &&
	usage

On Tue, 2009-06-16 at 13:46 -0700, Jeremy Fitzhardinge wrote:
> >
> >   retry:
> > -	wait_for_proc_work = thread->transaction_stack == NULL&&
> > -				list_empty(&thread->todo);
> > +	if (list_empty(&thread->todo)&&  thread->transaction_stack == NULL)
> > +		wait_for_proc_work = 1;
> >    
> 
> I think the original looks cleaner (in both cases).  Assigning a 
> variable with the result of a boolean expression is perfectly 
> reasonable.  Perhaps change the type of the variable to "bool" to make 
> it a bit clearer what's going on.

I agree it's reasonable in some cases.. The reason I changed this is
because at first glance I didn't know what those lines were suppose to
do. The equals signs all bleed together combined with the length of the
statement makes it not match other similar usage. The if statement just
makes the whole thing explicit.

Not to mention this code is a mess, very dense, and has little or no
comments. Anything that can be done to make the code more clear, seem
like a cleanup to me.

As for using "bool" , AFAIK that's only part of C++ ..

Daniel

--
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/