All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf tools: New function to parse string representing size in bytes
@ 2009-11-15  3:50 Hitoshi Mitake
  2009-11-15  5:11 ` Frederic Weisbecker
  0 siblings, 1 reply; 14+ messages in thread
From: Hitoshi Mitake @ 2009-11-15  3:50 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Hitoshi Mitake, Peter Zijlstra, Paul Mackerras,
	Frederic Weisbecker

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 <mitake@dcl.info.waseda.ac.jp>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] perf tools: New function to parse string representing size in bytes
  2009-11-15  3:50 [PATCH] perf tools: New function to parse string representing size in bytes Hitoshi Mitake
@ 2009-11-15  5:11 ` Frederic Weisbecker
  2009-11-15  8:08   ` Ingo Molnar
  0 siblings, 1 reply; 14+ messages in thread
From: Frederic Weisbecker @ 2009-11-15  5:11 UTC (permalink / raw)
  To: Hitoshi Mitake; +Cc: Ingo Molnar, linux-kernel, Peter Zijlstra, Paul Mackerras

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 <mitake@dcl.info.waseda.ac.jp>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> ---
>  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.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] perf tools: New function to parse string representing size in bytes
  2009-11-15  5:11 ` Frederic Weisbecker
@ 2009-11-15  8:08   ` Ingo Molnar
  2009-11-15  8:15     ` Hitoshi Mitake
  0 siblings, 1 reply; 14+ messages in thread
From: Ingo Molnar @ 2009-11-15  8:08 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Hitoshi Mitake, linux-kernel, Peter Zijlstra, Paul Mackerras


* Frederic Weisbecker <fweisbec@gmail.com> 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 <mitake@dcl.info.waseda.ac.jp>
> > Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > Cc: Paul Mackerras <paulus@samba.org>
> > Cc: Frederic Weisbecker <fweisbec@gmail.com>
> > ---
> >  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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] perf tools: New function to parse string representing size in bytes
  2009-11-15  8:08   ` Ingo Molnar
@ 2009-11-15  8:15     ` Hitoshi Mitake
  2009-11-15  8:17       ` [PATCH v2] " Hitoshi Mitake
  0 siblings, 1 reply; 14+ messages in thread
From: Hitoshi Mitake @ 2009-11-15  8:15 UTC (permalink / raw)
  To: mingo; +Cc: fweisbec, linux-kernel, a.p.zijlstra, paulus

From: Ingo Molnar <mingo@elte.hu>
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 <fweisbec@gmail.com> 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 <mitake@dcl.info.waseda.ac.jp>
> > > Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > > Cc: Paul Mackerras <paulus@samba.org>
> > > Cc: Frederic Weisbecker <fweisbec@gmail.com>
> > > ---
> > >  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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2] perf tools: New function to parse string representing size in bytes
  2009-11-15  8:15     ` Hitoshi Mitake
@ 2009-11-15  8:17       ` Hitoshi Mitake
  2009-11-15  8:57         ` Ingo Molnar
  0 siblings, 1 reply; 14+ messages in thread
From: Hitoshi Mitake @ 2009-11-15  8:17 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Hitoshi Mitake, Peter Zijlstra, Paul Mackerras,
	Frederic Weisbecker

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 <mitake@dcl.info.waseda.ac.jp>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] perf tools: New function to parse string representing size in bytes
  2009-11-15  8:17       ` [PATCH v2] " Hitoshi Mitake
@ 2009-11-15  8:57         ` Ingo Molnar
  2009-11-15  9:52           ` Hitoshi Mitake
  0 siblings, 1 reply; 14+ messages in thread
From: Ingo Molnar @ 2009-11-15  8:57 UTC (permalink / raw)
  To: Hitoshi Mitake
  Cc: linux-kernel, Peter Zijlstra, Paul Mackerras, Frederic Weisbecker


* Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp> 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 <mitake@dcl.info.waseda.ac.jp>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> ---
>  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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] perf tools: New function to parse string representing size in bytes
  2009-11-15  8:57         ` Ingo Molnar
@ 2009-11-15  9:52           ` Hitoshi Mitake
  2009-11-15 10:19             ` [PATCH v3] " Hitoshi Mitake
  0 siblings, 1 reply; 14+ messages in thread
From: Hitoshi Mitake @ 2009-11-15  9:52 UTC (permalink / raw)
  To: mingo; +Cc: linux-kernel, a.p.zijlstra, paulus, fweisbec

From: Ingo Molnar <mingo@elte.hu>
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 <mitake@dcl.info.waseda.ac.jp> 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 <mitake@dcl.info.waseda.ac.jp>
> > Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > Cc: Paul Mackerras <paulus@samba.org>
> > Cc: Frederic Weisbecker <fweisbec@gmail.com>
> > ---
> >  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
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3] perf tools: New function to parse string representing size in bytes
  2009-11-15  9:52           ` Hitoshi Mitake
@ 2009-11-15 10:19             ` Hitoshi Mitake
  2009-11-15 10:28               ` Ingo Molnar
  0 siblings, 1 reply; 14+ messages in thread
From: Hitoshi Mitake @ 2009-11-15 10:19 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Hitoshi Mitake, Peter Zijlstra, Paul Mackerras,
	Frederic Weisbecker

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 <mitake@dcl.info.waseda.ac.jp>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] perf tools: New function to parse string representing size in bytes
  2009-11-15 10:19             ` [PATCH v3] " Hitoshi Mitake
@ 2009-11-15 10:28               ` Ingo Molnar
  2009-11-15 10:57                 ` Hitoshi Mitake
  0 siblings, 1 reply; 14+ messages in thread
From: Ingo Molnar @ 2009-11-15 10:28 UTC (permalink / raw)
  To: Hitoshi Mitake
  Cc: linux-kernel, Peter Zijlstra, Paul Mackerras, Frederic Weisbecker


* Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp> 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] perf tools: New function to parse string representing size in bytes
  2009-11-15 10:28               ` Ingo Molnar
@ 2009-11-15 10:57                 ` Hitoshi Mitake
  2009-11-15 11:04                   ` Ingo Molnar
  0 siblings, 1 reply; 14+ messages in thread
From: Hitoshi Mitake @ 2009-11-15 10:57 UTC (permalink / raw)
  To: mingo; +Cc: linux-kernel, a.p.zijlstra, paulus, fweisbec

From: Ingo Molnar <mingo@elte.hu>
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 <mitake@dcl.info.waseda.ac.jp> 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.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] perf tools: New function to parse string representing size in bytes
  2009-11-15 10:57                 ` Hitoshi Mitake
@ 2009-11-15 11:04                   ` Ingo Molnar
  2009-11-15 11:32                     ` Hitoshi Mitake
  0 siblings, 1 reply; 14+ messages in thread
From: Ingo Molnar @ 2009-11-15 11:04 UTC (permalink / raw)
  To: Hitoshi Mitake; +Cc: linux-kernel, a.p.zijlstra, paulus, fweisbec


* Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp> 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] perf tools: New function to parse string representing size in bytes
  2009-11-15 11:04                   ` Ingo Molnar
@ 2009-11-15 11:32                     ` Hitoshi Mitake
  2009-11-15 11:36                       ` [PATCH v4] " Hitoshi Mitake
  0 siblings, 1 reply; 14+ messages in thread
From: Hitoshi Mitake @ 2009-11-15 11:32 UTC (permalink / raw)
  To: mingo; +Cc: linux-kernel, a.p.zijlstra, paulus, fweisbec

From: Ingo Molnar <mingo@elte.hu>
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 <mitake@dcl.info.waseda.ac.jp> 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.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v4] perf tools: New function to parse string representing size in bytes
  2009-11-15 11:32                     ` Hitoshi Mitake
@ 2009-11-15 11:36                       ` Hitoshi Mitake
  2009-11-15 14:18                         ` [tip:perf/core] perf tools: Add new perf_atoll() " tip-bot for Hitoshi Mitake
  0 siblings, 1 reply; 14+ messages in thread
From: Hitoshi Mitake @ 2009-11-15 11:36 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Hitoshi Mitake, Peter Zijlstra, Paul Mackerras,
	Frederic Weisbecker

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 <mitake@dcl.info.waseda.ac.jp>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [tip:perf/core] perf tools: Add new perf_atoll() function to parse string representing size in bytes
  2009-11-15 11:36                       ` [PATCH v4] " Hitoshi Mitake
@ 2009-11-15 14:18                         ` tip-bot for Hitoshi Mitake
  0 siblings, 0 replies; 14+ messages in thread
From: tip-bot for Hitoshi Mitake @ 2009-11-15 14:18 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, paulus, hpa, mingo, a.p.zijlstra, mitake, fweisbec,
	tglx, mingo

Commit-ID:  d2fb8b4151a92223da6a84006f8f248ebeb6677d
Gitweb:     http://git.kernel.org/tip/d2fb8b4151a92223da6a84006f8f248ebeb6677d
Author:     Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
AuthorDate: Sun, 15 Nov 2009 20:36:53 +0900
Committer:  Ingo Molnar <mingo@elte.hu>
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 <mitake@dcl.info.waseda.ac.jp>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
LKML-Reference: <1258285013-4759-1-git-send-email-mitake@dcl.info.waseda.ac.jp>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 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)

^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2009-11-15 14:19 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-11-15  3:50 [PATCH] perf tools: New function to parse string representing size in bytes Hitoshi Mitake
2009-11-15  5:11 ` Frederic Weisbecker
2009-11-15  8:08   ` Ingo Molnar
2009-11-15  8:15     ` Hitoshi Mitake
2009-11-15  8:17       ` [PATCH v2] " Hitoshi Mitake
2009-11-15  8:57         ` Ingo Molnar
2009-11-15  9:52           ` Hitoshi Mitake
2009-11-15 10:19             ` [PATCH v3] " Hitoshi Mitake
2009-11-15 10:28               ` Ingo Molnar
2009-11-15 10:57                 ` Hitoshi Mitake
2009-11-15 11:04                   ` Ingo Molnar
2009-11-15 11:32                     ` Hitoshi Mitake
2009-11-15 11:36                       ` [PATCH v4] " Hitoshi Mitake
2009-11-15 14:18                         ` [tip:perf/core] perf tools: Add new perf_atoll() " tip-bot for Hitoshi Mitake

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.