From: Steven Rostedt <[email protected]>
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.
Cc: John Reiser <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
---
scripts/recordmcount.c | 48 ++++++++++++++++++++++++------------------------
scripts/recordmcount.h | 16 ++++++++--------
2 files changed, 32 insertions(+), 32 deletions(-)
diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
index f9f6f52..37afe0e 100644
--- a/scripts/recordmcount.c
+++ b/scripts/recordmcount.c
@@ -78,7 +78,7 @@ static off_t
ulseek(int const fd, off_t const offset, int const whence)
{
off_t const w = lseek(fd, offset, whence);
- if ((off_t)-1 == w) {
+ if (w == (off_t)-1) {
perror("lseek");
fail_file();
}
@@ -111,7 +111,7 @@ static void *
umalloc(size_t size)
{
void *const addr = malloc(size);
- if (0 == addr) {
+ if (addr == 0) {
fprintf(stderr, "malloc failed: %zu bytes\n", size);
fail_file();
}
@@ -136,7 +136,7 @@ static void *mmap_file(char const *fname)
void *addr;
fd_map = open(fname, O_RDWR);
- if (0 > fd_map || 0 > fstat(fd_map, &sb)) {
+ if (fd_map < 0 || fstat(fd_map, &sb) < 0) {
perror(fname);
fail_file();
}
@@ -147,7 +147,7 @@ static void *mmap_file(char const *fname)
addr = mmap(0, sb.st_size, PROT_READ|PROT_WRITE, MAP_PRIVATE,
fd_map, 0);
mmap_failed = 0;
- if (MAP_FAILED == addr) {
+ if (addr == MAP_FAILED) {
mmap_failed = 1;
addr = umalloc(sb.st_size);
uread(fd_map, addr, sb.st_size);
@@ -206,12 +206,12 @@ static uint32_t (*w2)(uint16_t);
static int
is_mcounted_section_name(char const *const txtname)
{
- return 0 == strcmp(".text", txtname) ||
- 0 == strcmp(".ref.text", txtname) ||
- 0 == strcmp(".sched.text", txtname) ||
- 0 == strcmp(".spinlock.text", txtname) ||
- 0 == strcmp(".irqentry.text", txtname) ||
- 0 == strcmp(".text.unlikely", txtname);
+ return strcmp(".text", txtname) == 0 ||
+ strcmp(".ref.text", txtname) == 0 ||
+ strcmp(".sched.text", txtname) == 0 ||
+ strcmp(".spinlock.text", txtname) == 0 ||
+ strcmp(".irqentry.text", txtname) == 0 ||
+ strcmp(".text.unlikely", txtname) == 0;
}
/* 32 bit and 64 bit are very similar */
@@ -270,7 +270,7 @@ do_file(char const *const fname)
fail_file();
} break;
case ELFDATA2LSB: {
- if (1 != *(unsigned char const *)&endian) {
+ if (*(unsigned char const *)&endian != 1) {
/* main() is big endian, file.o is little endian. */
w = w4rev;
w2 = w2rev;
@@ -278,7 +278,7 @@ do_file(char const *const fname)
}
} break;
case ELFDATA2MSB: {
- if (0 != *(unsigned char const *)&endian) {
+ if (*(unsigned char const *)&endian != 0) {
/* main() is little endian, file.o is big endian. */
w = w4rev;
w2 = w2rev;
@@ -286,9 +286,9 @@ do_file(char const *const fname)
}
} break;
} /* end switch */
- if (0 != memcmp(ELFMAG, ehdr->e_ident, SELFMAG)
- || ET_REL != w2(ehdr->e_type)
- || EV_CURRENT != ehdr->e_ident[EI_VERSION]) {
+ if (memcmp(ELFMAG, ehdr->e_ident, SELFMAG) != 0
+ || w2(ehdr->e_type) != ET_REL
+ || ehdr->e_ident[EI_VERSION] != EV_CURRENT) {
fprintf(stderr, "unrecognized ET_REL file %s\n", fname);
fail_file();
}
@@ -321,15 +321,15 @@ do_file(char const *const fname)
fail_file();
} break;
case ELFCLASS32: {
- if (sizeof(Elf32_Ehdr) != w2(ehdr->e_ehsize)
- || sizeof(Elf32_Shdr) != w2(ehdr->e_shentsize)) {
+ if (w2(ehdr->e_ehsize) != sizeof(Elf32_Ehdr)
+ || w2(ehdr->e_shentsize) != sizeof(Elf32_Shdr)) {
fprintf(stderr,
"unrecognized ET_REL file: %s\n", fname);
fail_file();
}
- if (EM_S390 == w2(ehdr->e_machine))
+ if (w2(ehdr->e_machine) == EM_S390)
reltype = R_390_32;
- if (EM_MIPS == w2(ehdr->e_machine)) {
+ if (w2(ehdr->e_machine) == EM_MIPS) {
reltype = R_MIPS_32;
is_fake_mcount32 = MIPS32_is_fake_mcount;
}
@@ -337,15 +337,15 @@ do_file(char const *const fname)
} break;
case ELFCLASS64: {
Elf64_Ehdr *const ghdr = (Elf64_Ehdr *)ehdr;
- if (sizeof(Elf64_Ehdr) != w2(ghdr->e_ehsize)
- || sizeof(Elf64_Shdr) != w2(ghdr->e_shentsize)) {
+ if (w2(ghdr->e_ehsize) != sizeof(Elf64_Ehdr)
+ || w2(ghdr->e_shentsize) != sizeof(Elf64_Shdr)) {
fprintf(stderr,
"unrecognized ET_REL file: %s\n", fname);
fail_file();
}
- if (EM_S390 == w2(ghdr->e_machine))
+ if (w2(ghdr->e_machine) == EM_S390)
reltype = R_390_64;
- if (EM_MIPS == w2(ghdr->e_machine)) {
+ if (w2(ghdr->e_machine) == EM_MIPS) {
reltype = R_MIPS_64;
Elf64_r_sym = MIPS64_r_sym;
Elf64_r_info = MIPS64_r_info;
@@ -371,7 +371,7 @@ main(int argc, char const *argv[])
}
/* Process each file in turn, allowing deep failure. */
- for (--argc, ++argv; 0 < argc; --argc, ++argv) {
+ for (--argc, ++argv; argc > 0; --argc, ++argv) {
int const sjval = setjmp(jmpenv);
int len;
diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
index baf187b..ac7b330 100644
--- a/scripts/recordmcount.h
+++ b/scripts/recordmcount.h
@@ -275,12 +275,12 @@ static uint_t *sift_rel_mcount(uint_t *mlocp,
Elf_Sym const *const symp =
&sym0[Elf_r_sym(relp)];
char const *symname = &str0[w(symp->st_name)];
- char const *mcount = '_' == gpfx ? "_mcount" : "mcount";
+ char const *mcount = gpfx == '_' ? "_mcount" : "mcount";
- if ('.' == symname[0])
+ if (symname[0] == '.')
++symname; /* ppc64 hack */
- if (0 == strcmp(mcount, symname) ||
- (altmcount && 0 == strcmp(altmcount, symname)))
+ if (strcmp(mcount, symname) == 0 ||
+ (altmcount && strcmp(altmcount, symname) == 0))
mcountsym = Elf_r_sym(relp);
}
@@ -290,7 +290,7 @@ static uint_t *sift_rel_mcount(uint_t *mlocp,
mrelp->r_offset = _w(offbase
+ ((void *)mlocp - (void *)mloc0));
Elf_r_info(mrelp, recsym, reltype);
- if (sizeof(Elf_Rela) == rel_entsize) {
+ if (rel_entsize == sizeof(Elf_Rela)) {
((Elf_Rela *)mrelp)->r_addend = addend;
*mlocp++ = 0;
} else
@@ -354,12 +354,12 @@ __has_rel_mcount(Elf_Shdr const *const relhdr, /* is SHT_REL or SHT_RELA */
Elf_Shdr const *const txthdr = &shdr0[w(relhdr->sh_info)];
char const *const txtname = &shstrtab[w(txthdr->sh_name)];
- if (0 == strcmp("__mcount_loc", txtname)) {
+ if (strcmp("__mcount_loc", txtname) == 0) {
fprintf(stderr, "warning: __mcount_loc already exists: %s\n",
fname);
succeed_file();
}
- if (SHT_PROGBITS != w(txthdr->sh_type) ||
+ if (w(txthdr->sh_type) != SHT_PROGBITS ||
!is_mcounted_section_name(txtname))
return NULL;
return txtname;
@@ -370,7 +370,7 @@ static char const *has_rel_mcount(Elf_Shdr const *const relhdr,
char const *const shstrtab,
char const *const fname)
{
- if (SHT_REL != w(relhdr->sh_type) && SHT_RELA != w(relhdr->sh_type))
+ if (w(relhdr->sh_type) != SHT_REL && w(relhdr->sh_type) != SHT_RELA)
return NULL;
return __has_rel_mcount(relhdr, shdr0, shstrtab, fname);
}
--
1.7.2.3
On Wed, 20 Apr 2011 22:28:26 -0400
Steven Rostedt <[email protected]> wrote:
> From: Steven Rostedt <[email protected]>
>
> The Linux style for comparing is:
>
> var == 1
> var > 0
It's both and both forms are commonly used. I don't care what ftrace
looks like but don't pedal bogus style. We have enough bogus style as it
is.
On Thu, 2011-04-21 at 09:46 +0100, Alan Cox wrote:
> On Wed, 20 Apr 2011 22:28:26 -0400
> Steven Rostedt <[email protected]> wrote:
>
> > From: Steven Rostedt <[email protected]>
> >
> > The Linux style for comparing is:
> >
> > var == 1
> > var > 0
>
> It's both and both forms are commonly used. I don't care what ftrace
> looks like but don't pedal bogus style. We have enough bogus style as it
> is.
I thought I read somewhere that this was the preferred method. But I
could be mistaking.
Anyway, the patch still stands, although I'll change the above line from
"Linux style" to "Linux ftrace style", as I'm the one that has to
maintain this code, and I prefer this method.
I translate: var == 1 as "var is one" so seeing "1 == var" my mind
translates that to "one is var" which just sounds funny. Every time I
see that notation I have to stop and think about it. I'm sure if I used
it enough that hesitation would vanish, but for now, I'll keep it as is.
I haven't done the "if (var = 1)" mistake since college.
-- Steve
On Thu, 2011-04-21 at 07:36 -0400, Steven Rostedt wrote:
> I haven't done the "if (var = 1)" mistake since college.
>
I have a very bad feeling that I just jinxed myself and within the year,
I'll submit a patch with that very mistake :-p
-- Steve
On 04/20/2011 07:28 PM, Steven Rostedt wrote:
> From: Steven Rostedt <[email protected]>
>
> 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.
> - 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.
--
John Reiser
On Fri, Apr 22, 2011 at 12:09 PM, John Reiser <[email protected]> wrote:
> 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.
>
Maybe str_eq? Or even just streq? And also just !strcmp(a,b).
On Fri, 2011-04-22 at 12:52 -0300, Thiago Farina wrote:
> On Fri, Apr 22, 2011 at 12:09 PM, John Reiser <[email protected]> wrote:
> > 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.
> >
>
> Maybe str_eq? Or even just streq? And also just !strcmp(a,b).
streq() is something I woudn't mind.
I've too often confused !strcmp(a,b) as "!streq()" which is not the
case. Which is why I always use strcmp(a,b) == 0, which to me I see the
'==' as eq. I also consider strcmp(a,b) != 0 as not equal. Again, the
mind that sees "==" and "!=" can just translate that to human language.
Where !strcmp() is just gibberish ;)
-- Steve
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 <[email protected]>
> >
> > 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
On 04/22/2011 09:13 AM, Steven Rostedt wrote:
> 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.
Using ">" can be confused visually with "->", and I want to reduce those
chances. I also prefer a style that is prefix oriented, and with constants
on the left. I mentally combine the left constant and the infix operator
into a special case prefix operator. This speeds my parsing because it
reduces stack depth and enables faster scanning. This is particularly
helpful when the constant is narrow and the other operand is wider.
--
John Reiser
On 04/22/2011 10:40 AM, John Reiser wrote:
> On 04/22/2011 09:13 AM, Steven Rostedt wrote:
>> 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.
>
> Using ">" can be confused visually with "->", and I want to reduce those
> chances. I also prefer a style that is prefix oriented, and with constants
> on the left. I mentally combine the left constant and the infix operator
> into a special case prefix operator. This speeds my parsing because it
> reduces stack depth and enables faster scanning. This is particularly
> helpful when the constant is narrow and the other operand is wider.
>
I think you will find you're significantly in the minority when it comes
to that preference.
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
Hi,
On Fri, Apr 22, 2011 at 1:05 PM, Steven Rostedt <[email protected]> wrote:
> On Fri, 2011-04-22 at 12:52 -0300, Thiago Farina wrote:
>> On Fri, Apr 22, 2011 at 12:09 PM, John Reiser <[email protected]> wrote:
>> > 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.
>> >
>>
>> Maybe str_eq? Or even just streq? And also just !strcmp(a,b).
>
> streq() is something I woudn't mind.
>
> I've too often confused !strcmp(a,b) as "!streq()" which is not the
> case. Which is why I always use strcmp(a,b) == 0, which to me I see the
> '==' as eq. I also consider strcmp(a,b) != 0 as not equal. Again, the
> mind that sees "==" and "!=" can just translate that to human language.
> Where !strcmp() is just gibberish ;)
I've sent a patch to linux-kernel adding streq macro as suggested and
copied you Steven.
Best regards,
On Tue, 2011-04-26 at 15:52 -0300, Thiago Farina wrote:
> Hi,
> I've sent a patch to linux-kernel adding streq macro as suggested and
> copied you Steven.
Yep, saw it. If it is accepted, then I'll start conversions to it.
-- Steve
Commit-ID: dd5477ff3ba978892014ea5f988cb1bf04aa505e
Gitweb: http://git.kernel.org/tip/dd5477ff3ba978892014ea5f988cb1bf04aa505e
Author: Steven Rostedt <[email protected]>
AuthorDate: Wed, 6 Apr 2011 13:21:17 -0400
Committer: Steven Rostedt <[email protected]>
CommitDate: Mon, 16 May 2011 14:38:51 -0400
ftrace/trivial: Clean up recordmcount.c to use Linux style comparisons
The Linux ftrace subsystem 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.
Cc: John Reiser <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Steven Rostedt <[email protected]>
---
scripts/recordmcount.c | 48 ++++++++++++++++++++++++------------------------
scripts/recordmcount.h | 16 ++++++++--------
2 files changed, 32 insertions(+), 32 deletions(-)
diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
index f9f6f52..37afe0e 100644
--- a/scripts/recordmcount.c
+++ b/scripts/recordmcount.c
@@ -78,7 +78,7 @@ static off_t
ulseek(int const fd, off_t const offset, int const whence)
{
off_t const w = lseek(fd, offset, whence);
- if ((off_t)-1 == w) {
+ if (w == (off_t)-1) {
perror("lseek");
fail_file();
}
@@ -111,7 +111,7 @@ static void *
umalloc(size_t size)
{
void *const addr = malloc(size);
- if (0 == addr) {
+ if (addr == 0) {
fprintf(stderr, "malloc failed: %zu bytes\n", size);
fail_file();
}
@@ -136,7 +136,7 @@ static void *mmap_file(char const *fname)
void *addr;
fd_map = open(fname, O_RDWR);
- if (0 > fd_map || 0 > fstat(fd_map, &sb)) {
+ if (fd_map < 0 || fstat(fd_map, &sb) < 0) {
perror(fname);
fail_file();
}
@@ -147,7 +147,7 @@ static void *mmap_file(char const *fname)
addr = mmap(0, sb.st_size, PROT_READ|PROT_WRITE, MAP_PRIVATE,
fd_map, 0);
mmap_failed = 0;
- if (MAP_FAILED == addr) {
+ if (addr == MAP_FAILED) {
mmap_failed = 1;
addr = umalloc(sb.st_size);
uread(fd_map, addr, sb.st_size);
@@ -206,12 +206,12 @@ static uint32_t (*w2)(uint16_t);
static int
is_mcounted_section_name(char const *const txtname)
{
- return 0 == strcmp(".text", txtname) ||
- 0 == strcmp(".ref.text", txtname) ||
- 0 == strcmp(".sched.text", txtname) ||
- 0 == strcmp(".spinlock.text", txtname) ||
- 0 == strcmp(".irqentry.text", txtname) ||
- 0 == strcmp(".text.unlikely", txtname);
+ return strcmp(".text", txtname) == 0 ||
+ strcmp(".ref.text", txtname) == 0 ||
+ strcmp(".sched.text", txtname) == 0 ||
+ strcmp(".spinlock.text", txtname) == 0 ||
+ strcmp(".irqentry.text", txtname) == 0 ||
+ strcmp(".text.unlikely", txtname) == 0;
}
/* 32 bit and 64 bit are very similar */
@@ -270,7 +270,7 @@ do_file(char const *const fname)
fail_file();
} break;
case ELFDATA2LSB: {
- if (1 != *(unsigned char const *)&endian) {
+ if (*(unsigned char const *)&endian != 1) {
/* main() is big endian, file.o is little endian. */
w = w4rev;
w2 = w2rev;
@@ -278,7 +278,7 @@ do_file(char const *const fname)
}
} break;
case ELFDATA2MSB: {
- if (0 != *(unsigned char const *)&endian) {
+ if (*(unsigned char const *)&endian != 0) {
/* main() is little endian, file.o is big endian. */
w = w4rev;
w2 = w2rev;
@@ -286,9 +286,9 @@ do_file(char const *const fname)
}
} break;
} /* end switch */
- if (0 != memcmp(ELFMAG, ehdr->e_ident, SELFMAG)
- || ET_REL != w2(ehdr->e_type)
- || EV_CURRENT != ehdr->e_ident[EI_VERSION]) {
+ if (memcmp(ELFMAG, ehdr->e_ident, SELFMAG) != 0
+ || w2(ehdr->e_type) != ET_REL
+ || ehdr->e_ident[EI_VERSION] != EV_CURRENT) {
fprintf(stderr, "unrecognized ET_REL file %s\n", fname);
fail_file();
}
@@ -321,15 +321,15 @@ do_file(char const *const fname)
fail_file();
} break;
case ELFCLASS32: {
- if (sizeof(Elf32_Ehdr) != w2(ehdr->e_ehsize)
- || sizeof(Elf32_Shdr) != w2(ehdr->e_shentsize)) {
+ if (w2(ehdr->e_ehsize) != sizeof(Elf32_Ehdr)
+ || w2(ehdr->e_shentsize) != sizeof(Elf32_Shdr)) {
fprintf(stderr,
"unrecognized ET_REL file: %s\n", fname);
fail_file();
}
- if (EM_S390 == w2(ehdr->e_machine))
+ if (w2(ehdr->e_machine) == EM_S390)
reltype = R_390_32;
- if (EM_MIPS == w2(ehdr->e_machine)) {
+ if (w2(ehdr->e_machine) == EM_MIPS) {
reltype = R_MIPS_32;
is_fake_mcount32 = MIPS32_is_fake_mcount;
}
@@ -337,15 +337,15 @@ do_file(char const *const fname)
} break;
case ELFCLASS64: {
Elf64_Ehdr *const ghdr = (Elf64_Ehdr *)ehdr;
- if (sizeof(Elf64_Ehdr) != w2(ghdr->e_ehsize)
- || sizeof(Elf64_Shdr) != w2(ghdr->e_shentsize)) {
+ if (w2(ghdr->e_ehsize) != sizeof(Elf64_Ehdr)
+ || w2(ghdr->e_shentsize) != sizeof(Elf64_Shdr)) {
fprintf(stderr,
"unrecognized ET_REL file: %s\n", fname);
fail_file();
}
- if (EM_S390 == w2(ghdr->e_machine))
+ if (w2(ghdr->e_machine) == EM_S390)
reltype = R_390_64;
- if (EM_MIPS == w2(ghdr->e_machine)) {
+ if (w2(ghdr->e_machine) == EM_MIPS) {
reltype = R_MIPS_64;
Elf64_r_sym = MIPS64_r_sym;
Elf64_r_info = MIPS64_r_info;
@@ -371,7 +371,7 @@ main(int argc, char const *argv[])
}
/* Process each file in turn, allowing deep failure. */
- for (--argc, ++argv; 0 < argc; --argc, ++argv) {
+ for (--argc, ++argv; argc > 0; --argc, ++argv) {
int const sjval = setjmp(jmpenv);
int len;
diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
index baf187b..ac7b330 100644
--- a/scripts/recordmcount.h
+++ b/scripts/recordmcount.h
@@ -275,12 +275,12 @@ static uint_t *sift_rel_mcount(uint_t *mlocp,
Elf_Sym const *const symp =
&sym0[Elf_r_sym(relp)];
char const *symname = &str0[w(symp->st_name)];
- char const *mcount = '_' == gpfx ? "_mcount" : "mcount";
+ char const *mcount = gpfx == '_' ? "_mcount" : "mcount";
- if ('.' == symname[0])
+ if (symname[0] == '.')
++symname; /* ppc64 hack */
- if (0 == strcmp(mcount, symname) ||
- (altmcount && 0 == strcmp(altmcount, symname)))
+ if (strcmp(mcount, symname) == 0 ||
+ (altmcount && strcmp(altmcount, symname) == 0))
mcountsym = Elf_r_sym(relp);
}
@@ -290,7 +290,7 @@ static uint_t *sift_rel_mcount(uint_t *mlocp,
mrelp->r_offset = _w(offbase
+ ((void *)mlocp - (void *)mloc0));
Elf_r_info(mrelp, recsym, reltype);
- if (sizeof(Elf_Rela) == rel_entsize) {
+ if (rel_entsize == sizeof(Elf_Rela)) {
((Elf_Rela *)mrelp)->r_addend = addend;
*mlocp++ = 0;
} else
@@ -354,12 +354,12 @@ __has_rel_mcount(Elf_Shdr const *const relhdr, /* is SHT_REL or SHT_RELA */
Elf_Shdr const *const txthdr = &shdr0[w(relhdr->sh_info)];
char const *const txtname = &shstrtab[w(txthdr->sh_name)];
- if (0 == strcmp("__mcount_loc", txtname)) {
+ if (strcmp("__mcount_loc", txtname) == 0) {
fprintf(stderr, "warning: __mcount_loc already exists: %s\n",
fname);
succeed_file();
}
- if (SHT_PROGBITS != w(txthdr->sh_type) ||
+ if (w(txthdr->sh_type) != SHT_PROGBITS ||
!is_mcounted_section_name(txtname))
return NULL;
return txtname;
@@ -370,7 +370,7 @@ static char const *has_rel_mcount(Elf_Shdr const *const relhdr,
char const *const shstrtab,
char const *const fname)
{
- if (SHT_REL != w(relhdr->sh_type) && SHT_RELA != w(relhdr->sh_type))
+ if (w(relhdr->sh_type) != SHT_REL && w(relhdr->sh_type) != SHT_RELA)
return NULL;
return __has_rel_mcount(relhdr, shdr0, shstrtab, fname);
}
Commit-ID: c6fece27dfa46abc8c4b87d94a8c2f8cd6c5d8b1
Gitweb: http://git.kernel.org/tip/c6fece27dfa46abc8c4b87d94a8c2f8cd6c5d8b1
Author: Steven Rostedt <[email protected]>
AuthorDate: Wed, 6 Apr 2011 13:21:17 -0400
Committer: Steven Rostedt <[email protected]>
CommitDate: Tue, 17 May 2011 10:41:30 -0400
ftrace/trivial: Clean up recordmcount.c to use Linux style comparisons
The Linux ftrace subsystem 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.
Cc: John Reiser <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Steven Rostedt <[email protected]>
---
scripts/recordmcount.c | 48 ++++++++++++++++++++++++------------------------
scripts/recordmcount.h | 16 ++++++++--------
2 files changed, 32 insertions(+), 32 deletions(-)
diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
index f9f6f52..37afe0e 100644
--- a/scripts/recordmcount.c
+++ b/scripts/recordmcount.c
@@ -78,7 +78,7 @@ static off_t
ulseek(int const fd, off_t const offset, int const whence)
{
off_t const w = lseek(fd, offset, whence);
- if ((off_t)-1 == w) {
+ if (w == (off_t)-1) {
perror("lseek");
fail_file();
}
@@ -111,7 +111,7 @@ static void *
umalloc(size_t size)
{
void *const addr = malloc(size);
- if (0 == addr) {
+ if (addr == 0) {
fprintf(stderr, "malloc failed: %zu bytes\n", size);
fail_file();
}
@@ -136,7 +136,7 @@ static void *mmap_file(char const *fname)
void *addr;
fd_map = open(fname, O_RDWR);
- if (0 > fd_map || 0 > fstat(fd_map, &sb)) {
+ if (fd_map < 0 || fstat(fd_map, &sb) < 0) {
perror(fname);
fail_file();
}
@@ -147,7 +147,7 @@ static void *mmap_file(char const *fname)
addr = mmap(0, sb.st_size, PROT_READ|PROT_WRITE, MAP_PRIVATE,
fd_map, 0);
mmap_failed = 0;
- if (MAP_FAILED == addr) {
+ if (addr == MAP_FAILED) {
mmap_failed = 1;
addr = umalloc(sb.st_size);
uread(fd_map, addr, sb.st_size);
@@ -206,12 +206,12 @@ static uint32_t (*w2)(uint16_t);
static int
is_mcounted_section_name(char const *const txtname)
{
- return 0 == strcmp(".text", txtname) ||
- 0 == strcmp(".ref.text", txtname) ||
- 0 == strcmp(".sched.text", txtname) ||
- 0 == strcmp(".spinlock.text", txtname) ||
- 0 == strcmp(".irqentry.text", txtname) ||
- 0 == strcmp(".text.unlikely", txtname);
+ return strcmp(".text", txtname) == 0 ||
+ strcmp(".ref.text", txtname) == 0 ||
+ strcmp(".sched.text", txtname) == 0 ||
+ strcmp(".spinlock.text", txtname) == 0 ||
+ strcmp(".irqentry.text", txtname) == 0 ||
+ strcmp(".text.unlikely", txtname) == 0;
}
/* 32 bit and 64 bit are very similar */
@@ -270,7 +270,7 @@ do_file(char const *const fname)
fail_file();
} break;
case ELFDATA2LSB: {
- if (1 != *(unsigned char const *)&endian) {
+ if (*(unsigned char const *)&endian != 1) {
/* main() is big endian, file.o is little endian. */
w = w4rev;
w2 = w2rev;
@@ -278,7 +278,7 @@ do_file(char const *const fname)
}
} break;
case ELFDATA2MSB: {
- if (0 != *(unsigned char const *)&endian) {
+ if (*(unsigned char const *)&endian != 0) {
/* main() is little endian, file.o is big endian. */
w = w4rev;
w2 = w2rev;
@@ -286,9 +286,9 @@ do_file(char const *const fname)
}
} break;
} /* end switch */
- if (0 != memcmp(ELFMAG, ehdr->e_ident, SELFMAG)
- || ET_REL != w2(ehdr->e_type)
- || EV_CURRENT != ehdr->e_ident[EI_VERSION]) {
+ if (memcmp(ELFMAG, ehdr->e_ident, SELFMAG) != 0
+ || w2(ehdr->e_type) != ET_REL
+ || ehdr->e_ident[EI_VERSION] != EV_CURRENT) {
fprintf(stderr, "unrecognized ET_REL file %s\n", fname);
fail_file();
}
@@ -321,15 +321,15 @@ do_file(char const *const fname)
fail_file();
} break;
case ELFCLASS32: {
- if (sizeof(Elf32_Ehdr) != w2(ehdr->e_ehsize)
- || sizeof(Elf32_Shdr) != w2(ehdr->e_shentsize)) {
+ if (w2(ehdr->e_ehsize) != sizeof(Elf32_Ehdr)
+ || w2(ehdr->e_shentsize) != sizeof(Elf32_Shdr)) {
fprintf(stderr,
"unrecognized ET_REL file: %s\n", fname);
fail_file();
}
- if (EM_S390 == w2(ehdr->e_machine))
+ if (w2(ehdr->e_machine) == EM_S390)
reltype = R_390_32;
- if (EM_MIPS == w2(ehdr->e_machine)) {
+ if (w2(ehdr->e_machine) == EM_MIPS) {
reltype = R_MIPS_32;
is_fake_mcount32 = MIPS32_is_fake_mcount;
}
@@ -337,15 +337,15 @@ do_file(char const *const fname)
} break;
case ELFCLASS64: {
Elf64_Ehdr *const ghdr = (Elf64_Ehdr *)ehdr;
- if (sizeof(Elf64_Ehdr) != w2(ghdr->e_ehsize)
- || sizeof(Elf64_Shdr) != w2(ghdr->e_shentsize)) {
+ if (w2(ghdr->e_ehsize) != sizeof(Elf64_Ehdr)
+ || w2(ghdr->e_shentsize) != sizeof(Elf64_Shdr)) {
fprintf(stderr,
"unrecognized ET_REL file: %s\n", fname);
fail_file();
}
- if (EM_S390 == w2(ghdr->e_machine))
+ if (w2(ghdr->e_machine) == EM_S390)
reltype = R_390_64;
- if (EM_MIPS == w2(ghdr->e_machine)) {
+ if (w2(ghdr->e_machine) == EM_MIPS) {
reltype = R_MIPS_64;
Elf64_r_sym = MIPS64_r_sym;
Elf64_r_info = MIPS64_r_info;
@@ -371,7 +371,7 @@ main(int argc, char const *argv[])
}
/* Process each file in turn, allowing deep failure. */
- for (--argc, ++argv; 0 < argc; --argc, ++argv) {
+ for (--argc, ++argv; argc > 0; --argc, ++argv) {
int const sjval = setjmp(jmpenv);
int len;
diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
index baf187b..ac7b330 100644
--- a/scripts/recordmcount.h
+++ b/scripts/recordmcount.h
@@ -275,12 +275,12 @@ static uint_t *sift_rel_mcount(uint_t *mlocp,
Elf_Sym const *const symp =
&sym0[Elf_r_sym(relp)];
char const *symname = &str0[w(symp->st_name)];
- char const *mcount = '_' == gpfx ? "_mcount" : "mcount";
+ char const *mcount = gpfx == '_' ? "_mcount" : "mcount";
- if ('.' == symname[0])
+ if (symname[0] == '.')
++symname; /* ppc64 hack */
- if (0 == strcmp(mcount, symname) ||
- (altmcount && 0 == strcmp(altmcount, symname)))
+ if (strcmp(mcount, symname) == 0 ||
+ (altmcount && strcmp(altmcount, symname) == 0))
mcountsym = Elf_r_sym(relp);
}
@@ -290,7 +290,7 @@ static uint_t *sift_rel_mcount(uint_t *mlocp,
mrelp->r_offset = _w(offbase
+ ((void *)mlocp - (void *)mloc0));
Elf_r_info(mrelp, recsym, reltype);
- if (sizeof(Elf_Rela) == rel_entsize) {
+ if (rel_entsize == sizeof(Elf_Rela)) {
((Elf_Rela *)mrelp)->r_addend = addend;
*mlocp++ = 0;
} else
@@ -354,12 +354,12 @@ __has_rel_mcount(Elf_Shdr const *const relhdr, /* is SHT_REL or SHT_RELA */
Elf_Shdr const *const txthdr = &shdr0[w(relhdr->sh_info)];
char const *const txtname = &shstrtab[w(txthdr->sh_name)];
- if (0 == strcmp("__mcount_loc", txtname)) {
+ if (strcmp("__mcount_loc", txtname) == 0) {
fprintf(stderr, "warning: __mcount_loc already exists: %s\n",
fname);
succeed_file();
}
- if (SHT_PROGBITS != w(txthdr->sh_type) ||
+ if (w(txthdr->sh_type) != SHT_PROGBITS ||
!is_mcounted_section_name(txtname))
return NULL;
return txtname;
@@ -370,7 +370,7 @@ static char const *has_rel_mcount(Elf_Shdr const *const relhdr,
char const *const shstrtab,
char const *const fname)
{
- if (SHT_REL != w(relhdr->sh_type) && SHT_RELA != w(relhdr->sh_type))
+ if (w(relhdr->sh_type) != SHT_REL && w(relhdr->sh_type) != SHT_RELA)
return NULL;
return __has_rel_mcount(relhdr, shdr0, shstrtab, fname);
}