2009-11-15 03:50:49

by Hitoshi Mitake

[permalink] [raw]
Subject: [PATCH] perf tools: New function to parse string representing size in bytes

This patch modifies util/string.[ch] to add new function: bytesexp2int()
to parse string representing size in bytes.

Below is the description of bytesexp2int().

Parse (\d+)(b|B|kb|KB|mb|MB|gb|GB) (e.g. "256MB")
and return its numeric value. (e.g. 268435456)

The parameter str is not changed before and after calling,
but it changed temporally and internally for atoi().
So type of str is char *, not const char *.

Signed-off-by: Hitoshi Mitake <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
---
tools/perf/util/string.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++
tools/perf/util/string.h | 1 +
2 files changed, 90 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
index 04743d3..bbedb06 100644
--- a/tools/perf/util/string.c
+++ b/tools/perf/util/string.c
@@ -1,4 +1,5 @@
#include <string.h>
+#include <stdlib.h>
#include "string.h"

static int hex(char ch)
@@ -43,3 +44,91 @@ char *strxfrchar(char *s, char from, char to)

return s;
}
+
+static int digit(char ch)
+{
+ if ('0' <= ch && ch <= '9')
+ return 1;
+ return 0;
+}
+
+#define K 1024
+/*
+ * bytesexp2int()
+ * Parse (\d+)(b|B|kb|KB|mb|MB|gb|GB) (e.g. "256MB")
+ * and return its numeric value
+ *
+ * The parameter str is not changed before and after calling,
+ * but it changed temporally and internally for atoi().
+ * So type of str is char *, not const char *.
+ */
+int bytesexp2int(char *str)
+{
+ int i, unit = 1;
+ char shelter = '\0';
+ size_t length = -1;
+
+ if (!digit(str[0]))
+ goto err;
+
+ for (i = 1; i < (int)strlen(str); i++) {
+ switch (str[i]) {
+ case 'B':
+ case 'b':
+ break;
+ case 'K':
+ if (str[i + 1] != 'B')
+ goto err;
+ else
+ goto kilo;
+ case 'k':
+ if (str[i + 1] != 'b')
+ goto err;
+kilo:
+ unit = K;
+ break;
+ case 'M':
+ if (str[i + 1] != 'B')
+ goto err;
+ else
+ goto mega;
+ case 'm':
+ if (str[i + 1] != 'b')
+ goto err;
+mega:
+ unit = K * K;
+ break;
+ case 'G':
+ if (str[i + 1] != 'B')
+ goto err;
+ else
+ goto giga;
+ case 'g':
+ if (str[i + 1] != 'b')
+ goto err;
+giga:
+ unit = K * K * K;
+ break;
+ case '\0': /* only specified figures */
+ unit = 1;
+ break;
+ default:
+ if (!digit(str[i]))
+ goto err;
+ break;
+ }
+ }
+
+ shelter = str[i];
+ str[i] = (char)'\0';
+ length = atoi(str) * unit;
+ if (shelter != '\0')
+ str[i] = shelter;
+
+ goto end;
+
+err:
+ length = -1;
+end:
+ return length;
+}
diff --git a/tools/perf/util/string.h b/tools/perf/util/string.h
index 2c84bf6..adbf077 100644
--- a/tools/perf/util/string.h
+++ b/tools/perf/util/string.h
@@ -5,6 +5,7 @@

int hex2u64(const char *ptr, u64 *val);
char *strxfrchar(char *s, char from, char to);
+int bytesexp2int(char *str);

#define _STR(x) #x
#define STR(x) _STR(x)
--
1.6.5.2


2009-11-15 05:11:52

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: [PATCH] perf tools: New function to parse string representing size in bytes

On Sun, Nov 15, 2009 at 12:50:45PM +0900, Hitoshi Mitake wrote:
> This patch modifies util/string.[ch] to add new function: bytesexp2int()
> to parse string representing size in bytes.
>
> Below is the description of bytesexp2int().
>
> Parse (\d+)(b|B|kb|KB|mb|MB|gb|GB) (e.g. "256MB")
> and return its numeric value. (e.g. 268435456)
>
> The parameter str is not changed before and after calling,
> but it changed temporally and internally for atoi().
> So type of str is char *, not const char *.
>
> Signed-off-by: Hitoshi Mitake <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Frederic Weisbecker <[email protected]>
> ---
> tools/perf/util/string.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++
> tools/perf/util/string.h | 1 +
> 2 files changed, 90 insertions(+), 0 deletions(-)
>
> diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
> index 04743d3..bbedb06 100644
> --- a/tools/perf/util/string.c
> +++ b/tools/perf/util/string.c
> @@ -1,4 +1,5 @@
> #include <string.h>
> +#include <stdlib.h>
> #include "string.h"
>
> static int hex(char ch)
> @@ -43,3 +44,91 @@ char *strxfrchar(char *s, char from, char to)
>
> return s;
> }
> +
> +static int digit(char ch)
> +{
> + if ('0' <= ch && ch <= '9')
> + return 1;
> + return 0;
> +}



We have a "isdigit" macro in util.h already, despite the even already
existing isdigit from the libc. I don't know why we have that. I guess
it comes from git sources but I'm not sure why it has been reimplemented.

2009-11-15 08:08:46

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] perf tools: New function to parse string representing size in bytes


* Frederic Weisbecker <[email protected]> wrote:

> On Sun, Nov 15, 2009 at 12:50:45PM +0900, Hitoshi Mitake wrote:
> > This patch modifies util/string.[ch] to add new function: bytesexp2int()
> > to parse string representing size in bytes.
> >
> > Below is the description of bytesexp2int().
> >
> > Parse (\d+)(b|B|kb|KB|mb|MB|gb|GB) (e.g. "256MB")
> > and return its numeric value. (e.g. 268435456)
> >
> > The parameter str is not changed before and after calling,
> > but it changed temporally and internally for atoi().
> > So type of str is char *, not const char *.
> >
> > Signed-off-by: Hitoshi Mitake <[email protected]>
> > Cc: Peter Zijlstra <[email protected]>
> > Cc: Paul Mackerras <[email protected]>
> > Cc: Frederic Weisbecker <[email protected]>
> > ---
> > tools/perf/util/string.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++
> > tools/perf/util/string.h | 1 +
> > 2 files changed, 90 insertions(+), 0 deletions(-)
> >
> > diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
> > index 04743d3..bbedb06 100644
> > --- a/tools/perf/util/string.c
> > +++ b/tools/perf/util/string.c
> > @@ -1,4 +1,5 @@
> > #include <string.h>
> > +#include <stdlib.h>
> > #include "string.h"
> >
> > static int hex(char ch)
> > @@ -43,3 +44,91 @@ char *strxfrchar(char *s, char from, char to)
> >
> > return s;
> > }
> > +
> > +static int digit(char ch)
> > +{
> > + if ('0' <= ch && ch <= '9')
> > + return 1;
> > + return 0;
> > +}
>
>
>
> We have a "isdigit" macro in util.h already, despite the even already
> existing isdigit from the libc. I don't know why we have that. I guess
> it comes from git sources but I'm not sure why it has been
> reimplemented.

Git tends to be a lot saner when it comes to keeping library functions
sane, so i'd prefer if we kept and used the Git version.

Thanks,

Ingo

2009-11-15 08:15:53

by Hitoshi Mitake

[permalink] [raw]
Subject: Re: [PATCH] perf tools: New function to parse string representing size in bytes

From: Ingo Molnar <[email protected]>
Subject: Re: [PATCH] perf tools: New function to parse string representing size in bytes
Date: Sun, 15 Nov 2009 09:08:33 +0100

>
> * Frederic Weisbecker <[email protected]> wrote:
>
> > On Sun, Nov 15, 2009 at 12:50:45PM +0900, Hitoshi Mitake wrote:
> > > This patch modifies util/string.[ch] to add new function: bytesexp2int()
> > > to parse string representing size in bytes.
> > >
> > > Below is the description of bytesexp2int().
> > >
> > > Parse (\d+)(b|B|kb|KB|mb|MB|gb|GB) (e.g. "256MB")
> > > and return its numeric value. (e.g. 268435456)
> > >
> > > The parameter str is not changed before and after calling,
> > > but it changed temporally and internally for atoi().
> > > So type of str is char *, not const char *.
> > >
> > > Signed-off-by: Hitoshi Mitake <[email protected]>
> > > Cc: Peter Zijlstra <[email protected]>
> > > Cc: Paul Mackerras <[email protected]>
> > > Cc: Frederic Weisbecker <[email protected]>
> > > ---
> > > tools/perf/util/string.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++
> > > tools/perf/util/string.h | 1 +
> > > 2 files changed, 90 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
> > > index 04743d3..bbedb06 100644
> > > --- a/tools/perf/util/string.c
> > > +++ b/tools/perf/util/string.c
> > > @@ -1,4 +1,5 @@
> > > #include <string.h>
> > > +#include <stdlib.h>
> > > #include "string.h"
> > >
> > > static int hex(char ch)
> > > @@ -43,3 +44,91 @@ char *strxfrchar(char *s, char from, char to)
> > >
> > > return s;
> > > }
> > > +
> > > +static int digit(char ch)
> > > +{
> > > + if ('0' <= ch && ch <= '9')
> > > + return 1;
> > > + return 0;
> > > +}
> >
> >
> >
> > We have a "isdigit" macro in util.h already, despite the even already
> > existing isdigit from the libc. I don't know why we have that. I guess
> > it comes from git sources but I'm not sure why it has been
> > reimplemented.
>
> Git tends to be a lot saner when it comes to keeping library functions
> sane, so i'd prefer if we kept and used the Git version.

Thanks Frederic and Ingo,
I rewrote the patch according to your advice.
So I'll send the new one in this thread.

Hitoshi

2009-11-15 08:17:06

by Hitoshi Mitake

[permalink] [raw]
Subject: [PATCH v2] perf tools: New function to parse string representing size in bytes

This patch modifies util/string.[ch] to add new function: bytesexp2int()
to parse string representing size in bytes.

This is version 2. Acording to Frederic and Ingo's advice,
I removed static function digit() and rewrote to use
isdigit() macro of util.h.

Below is the description of bytesexp2int().

Parse (\d+)(b|B|kb|KB|mb|MB|gb|GB) (e.g. "256MB")
and return its numeric value. (e.g. 268435456)

The parameter str is not changed before and after calling,
but it changed temporally and internally for atoi().
So type of str is char *, not const char *.

Signed-off-by: Hitoshi Mitake <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
---
tools/perf/util/string.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++
tools/perf/util/string.h | 1 +
2 files changed, 83 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
index 04743d3..1ee7b17 100644
--- a/tools/perf/util/string.c
+++ b/tools/perf/util/string.c
@@ -1,4 +1,5 @@
#include <string.h>
+#include <stdlib.h>
#include "string.h"

static int hex(char ch)
@@ -43,3 +44,84 @@ char *strxfrchar(char *s, char from, char to)

return s;
}
+
+#define K 1024
+/*
+ * bytesexp2int()
+ * Parse (\d+)(b|B|kb|KB|mb|MB|gb|GB) (e.g. "256MB")
+ * and return its numeric value
+ *
+ * The parameter str is not changed before and after calling,
+ * but it changed temporally and internally for atoi().
+ * So type of str is char *, not const char *.
+ */
+int bytesexp2int(char *str)
+{
+ int i, unit = 1;
+ char shelter = '\0';
+ size_t length = -1;
+
+ if (!isdigit(str[0]))
+ goto err;
+
+ for (i = 1; i < (int)strlen(str); i++) {
+ switch (str[i]) {
+ case 'B':
+ case 'b':
+ break;
+ case 'K':
+ if (str[i + 1] != 'B')
+ goto err;
+ else
+ goto kilo;
+ case 'k':
+ if (str[i + 1] != 'b')
+ goto err;
+kilo:
+ unit = K;
+ break;
+ case 'M':
+ if (str[i + 1] != 'B')
+ goto err;
+ else
+ goto mega;
+ case 'm':
+ if (str[i + 1] != 'b')
+ goto err;
+mega:
+ unit = K * K;
+ break;
+ case 'G':
+ if (str[i + 1] != 'B')
+ goto err;
+ else
+ goto giga;
+ case 'g':
+ if (str[i + 1] != 'b')
+ goto err;
+giga:
+ unit = K * K * K;
+ break;
+ case '\0': /* only specified figures */
+ unit = 1;
+ break;
+ default:
+ if (!isdigit(str[i]))
+ goto err;
+ break;
+ }
+ }
+
+ shelter = str[i];
+ str[i] = (char)'\0';
+ length = atoi(str) * unit;
+ if (shelter != '\0')
+ str[i] = shelter;
+
+ goto end;
+
+err:
+ length = -1;
+end:
+ return length;
+}
diff --git a/tools/perf/util/string.h b/tools/perf/util/string.h
index 2c84bf6..adbf077 100644
--- a/tools/perf/util/string.h
+++ b/tools/perf/util/string.h
@@ -5,6 +5,7 @@

int hex2u64(const char *ptr, u64 *val);
char *strxfrchar(char *s, char from, char to);
+int bytesexp2int(char *str);

#define _STR(x) #x
#define STR(x) _STR(x)
--
1.6.5.2

2009-11-15 08:57:21

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v2] perf tools: New function to parse string representing size in bytes


* Hitoshi Mitake <[email protected]> wrote:

> This patch modifies util/string.[ch] to add new function: bytesexp2int()
> to parse string representing size in bytes.
>
> This is version 2. Acording to Frederic and Ingo's advice,
> I removed static function digit() and rewrote to use
> isdigit() macro of util.h.
>
> Below is the description of bytesexp2int().
>
> Parse (\d+)(b|B|kb|KB|mb|MB|gb|GB) (e.g. "256MB")
> and return its numeric value. (e.g. 268435456)
>
> The parameter str is not changed before and after calling,
> but it changed temporally and internally for atoi().
> So type of str is char *, not const char *.
>
> Signed-off-by: Hitoshi Mitake <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Frederic Weisbecker <[email protected]>
> ---
> tools/perf/util/string.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++
> tools/perf/util/string.h | 1 +
> 2 files changed, 83 insertions(+), 0 deletions(-)

Please run checkpatch on patches in the future, for this patch it shows
this small problem:

ERROR: trailing whitespace
#68: FILE: tools/perf/util/string.c:66:
+^I$

> diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
> index 04743d3..1ee7b17 100644
> --- a/tools/perf/util/string.c
> +++ b/tools/perf/util/string.c
> @@ -1,4 +1,5 @@
> #include <string.h>
> +#include <stdlib.h>
> #include "string.h"
>
> static int hex(char ch)
> @@ -43,3 +44,84 @@ char *strxfrchar(char *s, char from, char to)
>
> return s;
> }
> +
> +#define K 1024
> +/*
> + * bytesexp2int()
> + * Parse (\d+)(b|B|kb|KB|mb|MB|gb|GB) (e.g. "256MB")
> + * and return its numeric value
> + *
> + * The parameter str is not changed before and after calling,
> + * but it changed temporally and internally for atoi().
> + * So type of str is char *, not const char *.
> + */
> +int bytesexp2int(char *str)

why not to longlong?

> +{
> + int i, unit = 1;
> + char shelter = '\0';
> + size_t length = -1;
> +
> + if (!isdigit(str[0]))
> + goto err;
> +
> + for (i = 1; i < (int)strlen(str); i++) {

This extra '(int)' cast should be eliminated. In the kernel we avoid
unnecessary casts - here i suspect it can be done by changing 'int i' to
'unsigned int i'.

> + switch (str[i]) {
> + case 'B':
> + case 'b':
> + break;
> + case 'K':
> + if (str[i + 1] != 'B')
> + goto err;
> + else
> + goto kilo;
> + case 'k':
> + if (str[i + 1] != 'b')
> + goto err;
> +kilo:
> + unit = K;
> + break;
> + case 'M':
> + if (str[i + 1] != 'B')
> + goto err;
> + else
> + goto mega;
> + case 'm':
> + if (str[i + 1] != 'b')
> + goto err;
> +mega:
> + unit = K * K;
> + break;
> + case 'G':
> + if (str[i + 1] != 'B')
> + goto err;
> + else
> + goto giga;
> + case 'g':
> + if (str[i + 1] != 'b')
> + goto err;
> +giga:
> + unit = K * K * K;
> + break;
> + case '\0': /* only specified figures */
> + unit = 1;
> + break;
> + default:
> + if (!isdigit(str[i]))
> + goto err;
> + break;

(Might want to add tera as well, just in case this gets librarized into
the rest of the kernel some day.)

> + }
> + }
> +
> + shelter = str[i];
> + str[i] = (char)'\0';

a spurious type cast again - they should really be avoided. Here it's
unnecessary, character literals should auto-convert to the proper type
just fine.


> + length = atoi(str) * unit;
> + if (shelter != '\0')
> + str[i] = shelter;
> +
> + goto end;
> +
> +err:
> + length = -1;
> +end:
> + return length;
> +}

small naming nit, please use 'out_err:' and 'out:' labels. (which is how
we generally name such exception exit points)


Thanks,

Ingo

2009-11-15 09:52:21

by Hitoshi Mitake

[permalink] [raw]
Subject: Re: [PATCH v2] perf tools: New function to parse string representing size in bytes

From: Ingo Molnar <[email protected]>
Subject: Re: [PATCH v2] perf tools: New function to parse string representing size in bytes
Date: Sun, 15 Nov 2009 09:57:11 +0100

Thanks for your review, and sorry for my lots of fault...
I'll send version 3 later.

> * Hitoshi Mitake <[email protected]> wrote:
>
> > This patch modifies util/string.[ch] to add new function: bytesexp2int()
> > to parse string representing size in bytes.
> >
> > This is version 2. Acording to Frederic and Ingo's advice,
> > I removed static function digit() and rewrote to use
> > isdigit() macro of util.h.
> >
> > Below is the description of bytesexp2int().
> >
> > Parse (\d+)(b|B|kb|KB|mb|MB|gb|GB) (e.g. "256MB")
> > and return its numeric value. (e.g. 268435456)
> >
> > The parameter str is not changed before and after calling,
> > but it changed temporally and internally for atoi().
> > So type of str is char *, not const char *.
> >
> > Signed-off-by: Hitoshi Mitake <[email protected]>
> > Cc: Peter Zijlstra <[email protected]>
> > Cc: Paul Mackerras <[email protected]>
> > Cc: Frederic Weisbecker <[email protected]>
> > ---
> > tools/perf/util/string.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++
> > tools/perf/util/string.h | 1 +
> > 2 files changed, 83 insertions(+), 0 deletions(-)
>
> Please run checkpatch on patches in the future, for this patch it shows
> this small problem:
>
> ERROR: trailing whitespace
> #68: FILE: tools/perf/util/string.c:66:
> +^I$
>
> > diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
> > index 04743d3..1ee7b17 100644
> > --- a/tools/perf/util/string.c
> > +++ b/tools/perf/util/string.c
> > @@ -1,4 +1,5 @@
> > #include <string.h>
> > +#include <stdlib.h>
> > #include "string.h"
> >
> > static int hex(char ch)
> > @@ -43,3 +44,84 @@ char *strxfrchar(char *s, char from, char to)
> >
> > return s;
> > }
> > +
> > +#define K 1024
> > +/*
> > + * bytesexp2int()
> > + * Parse (\d+)(b|B|kb|KB|mb|MB|gb|GB) (e.g. "256MB")
> > + * and return its numeric value
> > + *
> > + * The parameter str is not changed before and after calling,
> > + * but it changed temporally and internally for atoi().
> > + * So type of str is char *, not const char *.
> > + */
> > +int bytesexp2int(char *str)
>
> why not to longlong?
>
> > +{
> > + int i, unit = 1;
> > + char shelter = '\0';
> > + size_t length = -1;
> > +
> > + if (!isdigit(str[0]))
> > + goto err;
> > +
> > + for (i = 1; i < (int)strlen(str); i++) {
>
> This extra '(int)' cast should be eliminated. In the kernel we avoid
> unnecessary casts - here i suspect it can be done by changing 'int i' to
> 'unsigned int i'.
>
> > + switch (str[i]) {
> > + case 'B':
> > + case 'b':
> > + break;
> > + case 'K':
> > + if (str[i + 1] != 'B')
> > + goto err;
> > + else
> > + goto kilo;
> > + case 'k':
> > + if (str[i + 1] != 'b')
> > + goto err;
> > +kilo:
> > + unit = K;
> > + break;
> > + case 'M':
> > + if (str[i + 1] != 'B')
> > + goto err;
> > + else
> > + goto mega;
> > + case 'm':
> > + if (str[i + 1] != 'b')
> > + goto err;
> > +mega:
> > + unit = K * K;
> > + break;
> > + case 'G':
> > + if (str[i + 1] != 'B')
> > + goto err;
> > + else
> > + goto giga;
> > + case 'g':
> > + if (str[i + 1] != 'b')
> > + goto err;
> > +giga:
> > + unit = K * K * K;
> > + break;
> > + case '\0': /* only specified figures */
> > + unit = 1;
> > + break;
> > + default:
> > + if (!isdigit(str[i]))
> > + goto err;
> > + break;
>
> (Might want to add tera as well, just in case this gets librarized into
> the rest of the kernel some day.)
>
> > + }
> > + }
> > +
> > + shelter = str[i];
> > + str[i] = (char)'\0';
>
> a spurious type cast again - they should really be avoided. Here it's
> unnecessary, character literals should auto-convert to the proper type
> just fine.
>
>
> > + length = atoi(str) * unit;
> > + if (shelter != '\0')
> > + str[i] = shelter;
> > +
> > + goto end;
> > +
> > +err:
> > + length = -1;
> > +end:
> > + return length;
> > +}
>
> small naming nit, please use 'out_err:' and 'out:' labels. (which is how
> we generally name such exception exit points)
>
>
> Thanks,
>
> Ingo
>

2009-11-15 10:19:08

by Hitoshi Mitake

[permalink] [raw]
Subject: [PATCH v3] perf tools: New function to parse string representing size in bytes

This patch modifies util/string.[ch] to add new function: bytesexp2int()
to parse string representing size in bytes.

This patch is version 3. According to Ingo's advice,
I fixed some points which violate kernel coding rule.

And I found that atoi()'s kindness.
When atoi() meets character not digit, this stops reading argument and
starts converting. e.g. atoi("1TB") -> 1
So I changed parameter type of bytesexp2int() from char * to const char *.
I think this is more natural style than old one.

This function parse (\d+)(b|B|kb|KB|mb|MB|gb|GB) (e.g. "256MB")
and return its numeric value. (e.g. 268435456)

Signed-off-by: Hitoshi Mitake <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
---
tools/perf/util/string.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++
tools/perf/util/string.h | 1 +
2 files changed, 85 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
index 04743d3..30ca7f3 100644
--- a/tools/perf/util/string.c
+++ b/tools/perf/util/string.c
@@ -1,5 +1,7 @@
#include <string.h>
+#include <stdlib.h>
#include "string.h"
+#include "util.h"

static int hex(char ch)
{
@@ -43,3 +45,85 @@ char *strxfrchar(char *s, char from, char to)

return s;
}
+
+#define K 1024
+/*
+ * bytesexp2int()
+ * Parse (\d+)(b|B|kb|KB|mb|MB|gb|GB|tb|TB) (e.g. "256MB")
+ * and return its numeric value
+ */
+long long int bytesexp2int(const char *str)
+{
+ unsigned int i;
+ long long int length = -1, unit = 1;
+
+ if (!isdigit(str[0]))
+ goto out_err;
+
+ for (i = 1; i < strlen(str); i++) {
+ switch (str[i]) {
+ case 'B':
+ case 'b':
+ break;
+ case 'K':
+ if (str[i + 1] != 'B')
+ goto out_err;
+ else
+ goto kilo;
+ case 'k':
+ if (str[i + 1] != 'b')
+ goto out_err;
+kilo:
+ unit = K;
+ break;
+ case 'M':
+ if (str[i + 1] != 'B')
+ goto out_err;
+ else
+ goto mega;
+ case 'm':
+ if (str[i + 1] != 'b')
+ goto out_err;
+mega:
+ unit = K * K;
+ break;
+ case 'G':
+ if (str[i + 1] != 'B')
+ goto out_err;
+ else
+ goto giga;
+ case 'g':
+ if (str[i + 1] != 'b')
+ goto out_err;
+giga:
+ unit = K * K * K;
+ break;
+ case 'T':
+ if (str[i + 1] != 'B')
+ goto out_err;
+ else
+ goto tera;
+ case 't':
+ if (str[i + 1] != 'b')
+ goto out_err;
+tera:
+ unit = (long long int)K * K * K * K;
+ break;
+ case '\0': /* only specified figures */
+ unit = 1;
+ break;
+ default:
+ if (!isdigit(str[i]))
+ goto out_err;
+ break;
+ }
+ }
+
+ length = atoll(str) * unit;
+ goto out;
+
+out_err:
+ length = -1;
+out:
+ return length;
+}
diff --git a/tools/perf/util/string.h b/tools/perf/util/string.h
index 2c84bf6..7e32233 100644
--- a/tools/perf/util/string.h
+++ b/tools/perf/util/string.h
@@ -5,6 +5,7 @@

int hex2u64(const char *ptr, u64 *val);
char *strxfrchar(char *s, char from, char to);
+long long int bytesexp2int(const char *str);

#define _STR(x) #x
#define STR(x) _STR(x)
--
1.6.5.2

2009-11-15 10:29:03

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v3] perf tools: New function to parse string representing size in bytes


* Hitoshi Mitake <[email protected]> wrote:

> + * bytesexp2int()

i suspect this could be named 'bytestring2ll' ? Is 'bytesexp' an
existing naming in other libraries?

> + * Parse (\d+)(b|B|kb|KB|mb|MB|gb|GB|tb|TB) (e.g. "256MB")
> + * and return its numeric value
> + */
> +long long int bytesexp2int(const char *str)

we have type shortcuts for 'long long': s64.

> +{
> + unsigned int i;
> + long long int length = -1, unit = 1;

s64 too i suspect.

> +tera:
> + unit = (long long int)K * K * K * K;

Note, you can probably avoid this type cast if you define the 'K'
literal as 1024LL or so.

> + length = atoll(str) * unit;

we might want to take a naming clue here and name this new function as
atoll_exp(), to signal that it works like atoll, just with an extension
for KB/MB/GB/etc. expressions?

Ingo

2009-11-15 10:57:12

by Hitoshi Mitake

[permalink] [raw]
Subject: Re: [PATCH v3] perf tools: New function to parse string representing size in bytes

From: Ingo Molnar <[email protected]>
Subject: Re: [PATCH v3] perf tools: New function to parse string representing size in bytes
Date: Sun, 15 Nov 2009 11:28:59 +0100

>
> * Hitoshi Mitake <[email protected]> wrote:
>
> > + * bytesexp2int()
>
> i suspect this could be named 'bytestring2ll' ? Is 'bytesexp' an
> existing naming in other libraries?

Sorry, I've forgot to rename.
And bytesexp may not exist in other libraries.
I couldn't create nice name for the notion number + unit in byes.

>
> > + * Parse (\d+)(b|B|kb|KB|mb|MB|gb|GB|tb|TB) (e.g. "256MB")
> > + * and return its numeric value
> > + */
> > +long long int bytesexp2int(const char *str)
>
> we have type shortcuts for 'long long': s64.
Thanks, I use s64. This is easy to type.

>
> > +{
> > + unsigned int i;
> > + long long int length = -1, unit = 1;
>
> s64 too i suspect.
>
> > +tera:
> > + unit = (long long int)K * K * K * K;
>
> Note, you can probably avoid this type cast if you define the 'K'
> literal as 1024LL or so.
I've forgot this notation, thanks.

>
> > + length = atoll(str) * unit;
>
> we might want to take a naming clue here and name this new function as
> atoll_exp(), to signal that it works like atoll, just with an extension
> for KB/MB/GB/etc. expressions?

Hm, how about "atoll_byteunit()"?
This may have no ambiguity.

2009-11-15 11:04:40

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v3] perf tools: New function to parse string representing size in bytes


* Hitoshi Mitake <[email protected]> wrote:

> > > + length = atoll(str) * unit;
> >
> > we might want to take a naming clue here and name this new function as
> > atoll_exp(), to signal that it works like atoll, just with an extension
> > for KB/MB/GB/etc. expressions?
>
> Hm, how about "atoll_byteunit()"?
> This may have no ambiguity.

I'd suggest to name it in a generic way, in case we want to add other
convenience conversions, hm?

Perhaps simply perf_atoll() - an extended version that understands byte
units (and potentially more in the future).

Ingo

2009-11-15 11:32:29

by Hitoshi Mitake

[permalink] [raw]
Subject: Re: [PATCH v3] perf tools: New function to parse string representing size in bytes

From: Ingo Molnar <[email protected]>
Subject: Re: [PATCH v3] perf tools: New function to parse string representing size in bytes
Date: Sun, 15 Nov 2009 12:04:32 +0100

>
> * Hitoshi Mitake <[email protected]> wrote:
>
> > > > + length = atoll(str) * unit;
> > >
> > > we might want to take a naming clue here and name this new function as
> > > atoll_exp(), to signal that it works like atoll, just with an extension
> > > for KB/MB/GB/etc. expressions?
> >
> > Hm, how about "atoll_byteunit()"?
> > This may have no ambiguity.
>
> I'd suggest to name it in a generic way, in case we want to add other
> convenience conversions, hm?
>
> Perhaps simply perf_atoll() - an extended version that understands byte
> units (and potentially more in the future).

OK. I name the function perf_atoll().
I'll send the patch later.

2009-11-15 11:36:56

by Hitoshi Mitake

[permalink] [raw]
Subject: [PATCH v4] perf tools: New function to parse string representing size in bytes

This patch modifies util/string.[ch] to add new function: perf_atoll()
to parse string representing size in bytes.

This patch is version 4. According to Ingo's advice,
I fixed some points which violate kernel coding rule.
And renamed new function to more general style.

This function parse (\d+)(b|B|kb|KB|mb|MB|gb|GB) (e.g. "256MB")
and return its numeric value. (e.g. 268435456)

Signed-off-by: Hitoshi Mitake <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
---
tools/perf/util/string.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++
tools/perf/util/string.h | 1 +
2 files changed, 85 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
index 04743d3..2270435 100644
--- a/tools/perf/util/string.c
+++ b/tools/perf/util/string.c
@@ -1,5 +1,7 @@
#include <string.h>
+#include <stdlib.h>
#include "string.h"
+#include "util.h"

static int hex(char ch)
{
@@ -43,3 +45,85 @@ char *strxfrchar(char *s, char from, char to)

return s;
}
+
+#define K 1024LL
+/*
+ * perf_atoll()
+ * Parse (\d+)(b|B|kb|KB|mb|MB|gb|GB|tb|TB) (e.g. "256MB")
+ * and return its numeric value
+ */
+s64 perf_atoll(const char *str)
+{
+ unsigned int i;
+ s64 length = -1, unit = 1;
+
+ if (!isdigit(str[0]))
+ goto out_err;
+
+ for (i = 1; i < strlen(str); i++) {
+ switch (str[i]) {
+ case 'B':
+ case 'b':
+ break;
+ case 'K':
+ if (str[i + 1] != 'B')
+ goto out_err;
+ else
+ goto kilo;
+ case 'k':
+ if (str[i + 1] != 'b')
+ goto out_err;
+kilo:
+ unit = K;
+ break;
+ case 'M':
+ if (str[i + 1] != 'B')
+ goto out_err;
+ else
+ goto mega;
+ case 'm':
+ if (str[i + 1] != 'b')
+ goto out_err;
+mega:
+ unit = K * K;
+ break;
+ case 'G':
+ if (str[i + 1] != 'B')
+ goto out_err;
+ else
+ goto giga;
+ case 'g':
+ if (str[i + 1] != 'b')
+ goto out_err;
+giga:
+ unit = K * K * K;
+ break;
+ case 'T':
+ if (str[i + 1] != 'B')
+ goto out_err;
+ else
+ goto tera;
+ case 't':
+ if (str[i + 1] != 'b')
+ goto out_err;
+tera:
+ unit = K * K * K * K;
+ break;
+ case '\0': /* only specified figures */
+ unit = 1;
+ break;
+ default:
+ if (!isdigit(str[i]))
+ goto out_err;
+ break;
+ }
+ }
+
+ length = atoll(str) * unit;
+ goto out;
+
+out_err:
+ length = -1;
+out:
+ return length;
+}
diff --git a/tools/perf/util/string.h b/tools/perf/util/string.h
index 2c84bf6..e50b07f 100644
--- a/tools/perf/util/string.h
+++ b/tools/perf/util/string.h
@@ -5,6 +5,7 @@

int hex2u64(const char *ptr, u64 *val);
char *strxfrchar(char *s, char from, char to);
+s64 perf_atoll(const char *str);

#define _STR(x) #x
#define STR(x) _STR(x)
--
1.6.5.2

2009-11-15 14:19:46

by Hitoshi Mitake

[permalink] [raw]
Subject: [tip:perf/core] perf tools: Add new perf_atoll() function to parse string representing size in bytes

Commit-ID: d2fb8b4151a92223da6a84006f8f248ebeb6677d
Gitweb: http://git.kernel.org/tip/d2fb8b4151a92223da6a84006f8f248ebeb6677d
Author: Hitoshi Mitake <[email protected]>
AuthorDate: Sun, 15 Nov 2009 20:36:53 +0900
Committer: Ingo Molnar <[email protected]>
CommitDate: Sun, 15 Nov 2009 14:54:23 +0100

perf tools: Add new perf_atoll() function to parse string representing size in bytes

This patch modifies util/string.[ch] to add new function:
perf_atoll() to parse string representing size in bytes.

This function parses (\d+)(b|B|kb|KB|mb|MB|gb|GB) (e.g. "256MB")
and returns its numeric value. (e.g. 268435456)

Signed-off-by: Hitoshi Mitake <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
tools/perf/util/string.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++
tools/perf/util/string.h | 1 +
2 files changed, 85 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
index 04743d3..2270435 100644
--- a/tools/perf/util/string.c
+++ b/tools/perf/util/string.c
@@ -1,5 +1,7 @@
#include <string.h>
+#include <stdlib.h>
#include "string.h"
+#include "util.h"

static int hex(char ch)
{
@@ -43,3 +45,85 @@ char *strxfrchar(char *s, char from, char to)

return s;
}
+
+#define K 1024LL
+/*
+ * perf_atoll()
+ * Parse (\d+)(b|B|kb|KB|mb|MB|gb|GB|tb|TB) (e.g. "256MB")
+ * and return its numeric value
+ */
+s64 perf_atoll(const char *str)
+{
+ unsigned int i;
+ s64 length = -1, unit = 1;
+
+ if (!isdigit(str[0]))
+ goto out_err;
+
+ for (i = 1; i < strlen(str); i++) {
+ switch (str[i]) {
+ case 'B':
+ case 'b':
+ break;
+ case 'K':
+ if (str[i + 1] != 'B')
+ goto out_err;
+ else
+ goto kilo;
+ case 'k':
+ if (str[i + 1] != 'b')
+ goto out_err;
+kilo:
+ unit = K;
+ break;
+ case 'M':
+ if (str[i + 1] != 'B')
+ goto out_err;
+ else
+ goto mega;
+ case 'm':
+ if (str[i + 1] != 'b')
+ goto out_err;
+mega:
+ unit = K * K;
+ break;
+ case 'G':
+ if (str[i + 1] != 'B')
+ goto out_err;
+ else
+ goto giga;
+ case 'g':
+ if (str[i + 1] != 'b')
+ goto out_err;
+giga:
+ unit = K * K * K;
+ break;
+ case 'T':
+ if (str[i + 1] != 'B')
+ goto out_err;
+ else
+ goto tera;
+ case 't':
+ if (str[i + 1] != 'b')
+ goto out_err;
+tera:
+ unit = K * K * K * K;
+ break;
+ case '\0': /* only specified figures */
+ unit = 1;
+ break;
+ default:
+ if (!isdigit(str[i]))
+ goto out_err;
+ break;
+ }
+ }
+
+ length = atoll(str) * unit;
+ goto out;
+
+out_err:
+ length = -1;
+out:
+ return length;
+}
diff --git a/tools/perf/util/string.h b/tools/perf/util/string.h
index 2c84bf6..e50b07f 100644
--- a/tools/perf/util/string.h
+++ b/tools/perf/util/string.h
@@ -5,6 +5,7 @@

int hex2u64(const char *ptr, u64 *val);
char *strxfrchar(char *s, char from, char to);
+s64 perf_atoll(const char *str);

#define _STR(x) #x
#define STR(x) _STR(x)