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: <1245424106.32124.4.camel@desktop>
Date:	Fri, 19 Jun 2009 08:08:26 -0700
From:	Daniel Walker <dwalker@...o99.com>
To:	Pavel Machek <pavel@....cz>
Cc:	Jeremy Fitzhardinge <jeremy@...p.org>,
	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 Fri, 2009-06-19 at 16:59 +0200, Pavel Machek wrote:
> >>   	int ret = 0;
> >> -	int wait_for_proc_work;
> >> +	int wait_for_proc_work = 0;
> >>
> >>   	if (*consumed == 0) {
> >>   		if (put_user(BR_NOOP, (uint32_t __user *)ptr))
> >> @@ -2155,8 +2155,8 @@ static int binder_thread_read(struct binder_proc *proc,
> >>   	}
> >>
> >>   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.
> >
> > (It would be a different matter if any of the expression had side-effects.)
> 
> Plus you have broken whitespace in there, changed performance
> characteristics (list_empty now done first) and with gotos used around
> that code I'm not at all sure transformation is equivalent.

The broken whitespace was only in replies to the original patch, not
sure how it made it in there. We already resolved most of what your
commenting. Unless you care to comment on a later email.

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/