linux-kernel - Re: [RFC][PATCH 01/11] ftrace/trivial: Clean up recordmcount.c to use Linux style comparisons

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]

Date:	Fri, 22 Apr 2011 12:13:46 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	John Reiser <jreiser@...wagon.com>
Cc:	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	"H. Peter Anvin" <hpa@...or.com>
Subject: Re: [RFC][PATCH 01/11] ftrace/trivial: Clean up recordmcount.c to
 use Linux style comparisons

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@...hat.com>
> > 
> > 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/