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

[Jeremy Fitzhardinge]

On 06/17/09 07:37, Daniel Walker wrote:
> 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.
>    

I definitely see your point, but the if() statement variant has the 
downside of only conditionally assigning the variable, and requiring it 
to be initialized separately.  I have a general code-cleanup rule to 
convert:

	foo = false;
	if (something_is_true())
		foo = true;

to

	foo = something_is_true();


Maybe a bit of reformatting and some tactical use of parens would help?

	wait_for_proc_work = (!thread->transaction_stack&&  list_empty(&thread->todo));

(I'm not normally a fan of NULL-as-false, but it reads OK here.)

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

No argument from me.  Not to mention that I have no idea from reading 
the code what the whole subsystem is for; "Android IPC Subsystem" 
doesn't tell me much, other than a gnawing feeling about having yet 
another IPC subsystem to deal with.

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

No, it is also C99, and becoming widely used in the kernel.

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