All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf: fix bug in isupper() and islower()
@ 2013-03-29 19:29 Sukadev Bhattiprolu
  2013-04-01  6:19 ` Namhyung Kim
  2013-05-31 11:12 ` [tip:perf/core] perf tools: Fix " tip-bot for Sukadev Bhattiprolu
  0 siblings, 2 replies; 4+ messages in thread
From: Sukadev Bhattiprolu @ 2013-03-29 19:29 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: linux-kernel

>From fd349681226bf7b27c9d0f72b0f3941b5aa94f78 Mon Sep 17 00:00:00 2001
From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Date: Fri, 29 Mar 2013 12:14:43 -0700
Subject: [PATCH] perf: fix bug in isupper() and islower()

One of the reasons 'perf test' is failing on Power appears to be due to
a bug in isupper().

isupper(c) and islower(c) should be checking 'c' against the mask 0x20.
Instead they are checking sane_ctype[c] which causes isupper() to be true
for lower case letters.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 tools/perf/util/util.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 09b4c26..2795ef6 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -219,8 +219,8 @@ extern unsigned char sane_ctype[256];
 #define isalpha(x) sane_istest(x,GIT_ALPHA)
 #define isalnum(x) sane_istest(x,GIT_ALPHA | GIT_DIGIT)
 #define isprint(x) sane_istest(x,GIT_PRINT)
-#define islower(x) (sane_istest(x,GIT_ALPHA) && sane_istest(x,0x20))
-#define isupper(x) (sane_istest(x,GIT_ALPHA) && !sane_istest(x,0x20))
+#define islower(x) (sane_istest(x,GIT_ALPHA) && (x & 0x20))
+#define isupper(x) (sane_istest(x,GIT_ALPHA) && !(x & 0x20))
 #define tolower(x) sane_case((unsigned char)(x), 0x20)
 #define toupper(x) sane_case((unsigned char)(x), 0)
 
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] perf: fix bug in isupper() and islower()
  2013-03-29 19:29 [PATCH] perf: fix bug in isupper() and islower() Sukadev Bhattiprolu
@ 2013-04-01  6:19 ` Namhyung Kim
  2013-04-02 14:16   ` Arnaldo Carvalho de Melo
  2013-05-31 11:12 ` [tip:perf/core] perf tools: Fix " tip-bot for Sukadev Bhattiprolu
  1 sibling, 1 reply; 4+ messages in thread
From: Namhyung Kim @ 2013-04-01  6:19 UTC (permalink / raw)
  To: Sukadev Bhattiprolu; +Cc: Arnaldo Carvalho de Melo, linux-kernel

Hi Sukadev,

On Fri, 29 Mar 2013 12:29:50 -0700, Sukadev Bhattiprolu wrote:
> From fd349681226bf7b27c9d0f72b0f3941b5aa94f78 Mon Sep 17 00:00:00 2001
> From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> Date: Fri, 29 Mar 2013 12:14:43 -0700
> Subject: [PATCH] perf: fix bug in isupper() and islower()
>
> One of the reasons 'perf test' is failing on Power appears to be due to
> a bug in isupper().
>
> isupper(c) and islower(c) should be checking 'c' against the mask 0x20.
> Instead they are checking sane_ctype[c] which causes isupper() to be true
> for lower case letters.

Indeed!

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung

>
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> ---
>  tools/perf/util/util.h |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
> index 09b4c26..2795ef6 100644
> --- a/tools/perf/util/util.h
> +++ b/tools/perf/util/util.h
> @@ -219,8 +219,8 @@ extern unsigned char sane_ctype[256];
>  #define isalpha(x) sane_istest(x,GIT_ALPHA)
>  #define isalnum(x) sane_istest(x,GIT_ALPHA | GIT_DIGIT)
>  #define isprint(x) sane_istest(x,GIT_PRINT)
> -#define islower(x) (sane_istest(x,GIT_ALPHA) && sane_istest(x,0x20))
> -#define isupper(x) (sane_istest(x,GIT_ALPHA) && !sane_istest(x,0x20))
> +#define islower(x) (sane_istest(x,GIT_ALPHA) && (x & 0x20))
> +#define isupper(x) (sane_istest(x,GIT_ALPHA) && !(x & 0x20))
>  #define tolower(x) sane_case((unsigned char)(x), 0x20)
>  #define toupper(x) sane_case((unsigned char)(x), 0)

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] perf: fix bug in isupper() and islower()
  2013-04-01  6:19 ` Namhyung Kim
@ 2013-04-02 14:16   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2013-04-02 14:16 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: Sukadev Bhattiprolu, linux-kernel

Em Mon, Apr 01, 2013 at 03:19:28PM +0900, Namhyung Kim escreveu:
> On Fri, 29 Mar 2013 12:29:50 -0700, Sukadev Bhattiprolu wrote:
> > Subject: [PATCH] perf: fix bug in isupper() and islower()

> > One of the reasons 'perf test' is failing on Power appears to be due to
> > a bug in isupper().

> > isupper(c) and islower(c) should be checking 'c' against the mask 0x20.
> > Instead they are checking sane_ctype[c] which causes isupper() to be true
> > for lower case letters.

> Indeed!
> Acked-by: Namhyung Kim <namhyung@kernel.org>

But can we try to use the same defs as for the kernel? Or at least sync
this code against git.git, that is where it comes from?

There we have:

#define islower(x) sane_iscase(x, 1)
#define isupper(x) sane_iscase(x, 0)

#define sane_istest(x,mask) ((sane_ctype[(unsigned char)(x)] & (mask)) != 0)

static inline int sane_iscase(int x, int is_lower)
{
        if (!sane_istest(x, GIT_ALPHA))
                return 0;

        if (is_lower)
                return (x & 0x20) != 0;
        else
                return (x & 0x20) == 0;
}
----------------------------------------------------------------------------

In the kernel we have instead:

include/linux/ctype.h

#define _U      0x01    /* upper */
#define __ismask(x) (_ctype[(int)(unsigned char)(x)])
#define isupper(c)      ((__ismask(c)&(_U)) != 0)

----------------------------------------------------------------------------

I'm merging this fix now, as it improves things, but this seemingly
needless speciations looks insane, would be great to have at least
tools/perf/  using the same stuff as its landlord, the kernel.

- Arnaldo

^ permalink raw reply	[flat|nested] 4+ messages in thread
* [tip:perf/core] perf tools: Fix bug in isupper() and islower()
  2013-03-29 19:29 [PATCH] perf: fix bug in isupper() and islower() Sukadev Bhattiprolu
  2013-04-01  6:19 ` Namhyung Kim
@ 2013-05-31 11:12 ` tip-bot for Sukadev Bhattiprolu
  1 sibling, 0 replies; 4+ messages in thread
From: tip-bot for Sukadev Bhattiprolu @ 2013-05-31 11:12 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: acme, linux-kernel, hpa, mingo, sukadev, tglx, namhyung

Commit-ID:  6956664a5c4c32d5aa48fe96d5e2421a3e3f72d5
Gitweb:     http://git.kernel.org/tip/6956664a5c4c32d5aa48fe96d5e2421a3e3f72d5
Author:     Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
AuthorDate: Fri, 29 Mar 2013 12:14:43 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 28 May 2013 16:23:51 +0300

perf tools: Fix bug in isupper() and islower()

One of the reasons 'perf test' is failing on Power appears to be due to
a bug in isupper().

isupper(c) and islower(c) should be checking 'c' against the mask 0x20.
Instead they are checking sane_ctype[c] which causes isupper() to be
true for lower case letters.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/r/20130329192950.GA9312@us.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/util.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index a45710b..7a484c9 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -221,8 +221,8 @@ extern unsigned char sane_ctype[256];
 #define isalpha(x) sane_istest(x,GIT_ALPHA)
 #define isalnum(x) sane_istest(x,GIT_ALPHA | GIT_DIGIT)
 #define isprint(x) sane_istest(x,GIT_PRINT)
-#define islower(x) (sane_istest(x,GIT_ALPHA) && sane_istest(x,0x20))
-#define isupper(x) (sane_istest(x,GIT_ALPHA) && !sane_istest(x,0x20))
+#define islower(x) (sane_istest(x,GIT_ALPHA) && (x & 0x20))
+#define isupper(x) (sane_istest(x,GIT_ALPHA) && !(x & 0x20))
 #define tolower(x) sane_case((unsigned char)(x), 0x20)
 #define toupper(x) sane_case((unsigned char)(x), 0)
 

^ permalink raw reply related	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-05-31 11:13 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-29 19:29 [PATCH] perf: fix bug in isupper() and islower() Sukadev Bhattiprolu
2013-04-01  6:19 ` Namhyung Kim
2013-04-02 14:16   ` Arnaldo Carvalho de Melo
2013-05-31 11:12 ` [tip:perf/core] perf tools: Fix " tip-bot for Sukadev Bhattiprolu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.