All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] perf: fix bug in isupper() and islower()
Date: Tue, 2 Apr 2013 11:16:04 -0300	[thread overview]
Message-ID: <20130402141604.GD1952@ghostprotocols.net> (raw)
In-Reply-To: <876206hixb.fsf@sejong.aot.lge.com>

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

  reply	other threads:[~2013-04-02 14:16 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2013-05-31 11:12 ` [tip:perf/core] perf tools: Fix " tip-bot for Sukadev Bhattiprolu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130402141604.GD1952@ghostprotocols.net \
    --to=acme@ghostprotocols.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=namhyung@kernel.org \
    --cc=sukadev@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.