Re: [RFC][PATCH 01/11] ftrace/trivial: Clean up recordmcount.c touse Linux style comparisons

[Steven Rostedt]

On Fri, 2011-04-22 at 08:09 -0700, John Reiser wrote:
> On 04/20/2011 07:28 PM, Steven Rostedt wrote:
> > From: Steven Rostedt <srostedt@xxxxxxxxxx>
> > 
> > The Linux style for comparing is:
> > 
> >   var == 1
> >   var > 0
> > 
> > and not:
> > 
> >   1 == var
> >   0 < var
> > 
> > It is considered that Linux developers are smart enough not to do the
> > 
> >   if (var = 1)
> > 
> > mistake.
> 
> It's not just a matter of 'smart', it's a matter of safety.
> For me it still catches a bug (typo, copy+paste, fumble in editor script, ...)
> every year or two.  Compilers haven't always warned, or the option to warn
> might be turned off.


I totally understand why it is used. But personally, it confuses my
train of thought when reading code. That notation is just a work around
to a deficiency in the C language. And I find the time spent trying to
decipher 0 == var takes up much more time than debugging if (var = 0).

Although, I have no idea why you choose the 0 < var, that totally
confuses me. It does not play any role in assignments. What bug is that
preventing? When I want to know if a variable is greater than zero, I
don't show it as zero less than the var.

> 
> > -	return 0 == strcmp(".text",           txtname) ||
> > +	return strcmp(".text",           txtname) == 0 ||
> 
> I consider "0==strcmp(" to be an idiom.  Too often "strcmp(...) == 0"
> overflows my mental stack because of the typographic width of the operands
> in the source code.  If you still object in this case then please consider
> using something like:
> 	#define strequ(a,b) (strcmp((a), (b)) == 0)
> or
> 	static int strequ(char const *a, char const *b)
> 	{
> 		return strcmp(a, b) == 0;
> 	}
> which names the idiom.

I'm all for making a streq. Perhaps that could be a nice clean up of the
kernel. It definitely makes it much easier to read and understand. But
that is for another time.

As I'm working on this code, and I'm the maintainer, I want to make sure
I can read and review the code easily. Anything that distracts me for
other people's personal taste, is something that I don't have the luxury
of time for. Thus, I'm keeping these patches as is for the time being.

-- Steve



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