From: Daniel Walker <dwalker@fifo99.com>
To: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: Greg Kroah-Hartman <greg@kroah.com>,
Brian Swetland <swetland@google.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/6] staging: android: binder: Remove some funny && usage
Date: Wed, 17 Jun 2009 07:37:49 -0700 [thread overview]
Message-ID: <1245249469.5982.251.camel@desktop> (raw)
In-Reply-To: <4A380494.6030506@goop.org>
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
next prev parent reply other threads:[~2009-06-17 14:37 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-06-12 18:51 [PATCH 1/6] staging: android: binder: Remove some funny && usage Daniel Walker
2009-06-12 18:51 ` [PATCH 2/6] staging: android: binder: move debugging mask into a macro Daniel Walker
2009-06-12 18:51 ` [PATCH 3/6] staging: android: binder: remove a predefine Daniel Walker
2009-06-12 18:51 ` [PATCH 4/6] staging: android: binder: add enum usage in function arguments Daniel Walker
2009-06-12 18:51 ` [PATCH 5/6] staging: android: binder: global variable cleanup Daniel Walker
2009-06-12 18:51 ` [PATCH 6/6] staging: android: binder: clean up for all the stat statments Daniel Walker
2009-06-16 20:46 ` [PATCH 1/6] staging: android: binder: Remove some funny && usage Jeremy Fitzhardinge
2009-06-17 14:37 ` Daniel Walker [this message]
2009-06-17 15:28 ` Jeremy Fitzhardinge
2009-06-17 16:08 ` Daniel Walker
2009-06-17 16:31 ` Jeremy Fitzhardinge
2009-06-17 21:26 ` Arve Hjønnevåg
2009-06-17 21:31 ` Daniel Walker
2009-06-19 19:20 ` Brian Swetland
2009-06-19 22:53 ` Daniel Walker
2009-06-20 0:13 ` Arve Hjønnevåg
2009-06-20 0:49 ` Daniel Walker
2009-06-20 18:48 ` Christoph Hellwig
2009-06-21 12:09 ` Marcel Holtmann
2009-06-25 4:09 ` Dianne Hackborn
2009-06-25 10:14 ` Marcel Holtmann
2009-06-25 11:34 ` Alan Cox
2009-06-25 13:24 ` Daniel Walker
2009-06-27 2:20 ` GeunSik Lim
2009-06-20 1:26 ` GeunSik Lim
2009-06-24 13:13 ` Daniel Walker
2009-06-24 22:14 ` Brian Swetland
2009-06-24 22:49 ` Daniel Walker
2009-06-24 23:05 ` Brian Swetland
2009-06-24 23:29 ` Daniel Walker
2009-06-24 23:37 ` Brian Swetland
2009-06-25 0:01 ` Linus Walleij
2009-06-25 0:20 ` Daniel Walker
2009-06-25 8:15 ` Alan Cox
2009-06-25 9:56 ` Marcel Holtmann
2009-06-17 21:38 ` Jeremy Fitzhardinge
2009-06-19 14:59 ` Pavel Machek
2009-06-19 15:08 ` Daniel Walker
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1245249469.5982.251.camel@desktop \
--to=dwalker@fifo99.com \
--cc=greg@kroah.com \
--cc=jeremy@goop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=swetland@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox