[Qemu-devel] [PATCH 0/2] Fix size default for qemu-img

[Qemu-devel] [PATCH 0/2] Fix size default for qemu-img

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Kevin pointed out that my chance to img_create()'s handling of the
image size, changed the previous default of byte for size if no suffix
was specified, since strtosz() defaults to MB.

This patch set introduces strtosz_suffix() and then changes
img_create() to use that instead, thereby restoring the old default
behavior.

Jes Sorensen (2):
  Introduce strtosz_suffix()
  Make img_create() use strtosz_suffix()

 cutils.c      |   17 ++++++++++++++---
 qemu-common.h |    7 +++++++
 qemu-img.c    |    2 +-
 3 files changed, 22 insertions(+), 4 deletions(-)

-- 
1.7.3.2

[Qemu-devel] [PATCH 1/2] Introduce strtosz_suffix()

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

This introduces strtosz_suffix() which allows the caller to specify a
default suffix in case the non default of MB is wanted.

strtosz() is kept as a wrapper for strtosz_suffix() which keeps it's
current default of MB.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 cutils.c      |   17 ++++++++++++++---
 qemu-common.h |    7 +++++++
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/cutils.c b/cutils.c
index 28089aa..1d24d9a 100644
--- a/cutils.c
+++ b/cutils.c
@@ -291,10 +291,10 @@ int fcntl_setfl(int fd, int flag)
  * value must be terminated by whitespace, ',' or '\0'. Return -1 on
  * error.
  */
-ssize_t strtosz(const char *nptr, char **end)
+ssize_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
 {
     ssize_t retval = -1;
-    char *endptr, c;
+    char *endptr, c, d;
     int mul_required = 0;
     double val, mul, integral, fraction;
 
@@ -313,10 +313,16 @@ ssize_t strtosz(const char *nptr, char **end)
      * part of a multi token argument.
      */
     c = *endptr;
+    d = c;
     if (isspace(c) || c == '\0' || c == ',') {
         c = 0;
+        if (default_suffix) {
+            d = default_suffix;
+        } else {
+            d = c;
+        }
     }
-    switch (c) {
+    switch (d) {
     case 'B':
     case 'b':
         mul = 1;
@@ -371,3 +377,8 @@ fail:
 
     return retval;
 }
+
+ssize_t strtosz(const char *nptr, char **end)
+{
+    return strtosz_suffix(nptr, end, 0);
+}
diff --git a/qemu-common.h b/qemu-common.h
index de82c2e..dc44cd6 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -149,7 +149,14 @@ time_t mktimegm(struct tm *tm);
 int qemu_fls(int i);
 int qemu_fdatasync(int fd);
 int fcntl_setfl(int fd, int flag);
+
+#define STRTOSZ_DEFSUFFIX_TB	'T'
+#define STRTOSZ_DEFSUFFIX_GB	'G'
+#define STRTOSZ_DEFSUFFIX_MB	'M'
+#define STRTOSZ_DEFSUFFIX_KB	'K'
+#define STRTOSZ_DEFSUFFIX_B	'B'
 ssize_t strtosz(const char *nptr, char **end);
+ssize_t strtosz_suffix(const char *nptr, char **end, const char);
 
 /* path.c */
 void init_paths(const char *prefix);
-- 
1.7.3.2

[Qemu-devel] [PATCH 2/2] Make img_create() use strtosz_suffix()

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

This reestablished the old default of using bytes as the default for
the size argument, and not MB as we do in pretty much every other
place.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 qemu-img.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 9a5e7e1..603bdb3 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -331,7 +331,7 @@ static int img_create(int argc, char **argv)
     /* Get image size, if specified */
     if (optind < argc) {
         ssize_t sval;
-        sval = strtosz(argv[optind++], NULL);
+        sval = strtosz_suffix(argv[optind++], NULL, STRTOSZ_DEFSUFFIX_B);
         if (sval < 0) {
             error("Invalid image size specified! You may use k, M, G or "
                   "T suffixes for ");
-- 
1.7.3.2

[Qemu-devel] [PATCH v4 0/2] Clean up img_create() and introduce strtosz_suffix()

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

This patch set introduces strtosz_suffix() which is needed to be able
to use strtosz parsing with a non MB default suffix. This is used to
clean up qemu-img.c:img_create().

Kevin asked me to rebase this instead of applying the other patches on
top, so please discard the previous versions. Sorry for the patch
noise.

Jes Sorensen (2):
  Introduce strtosz_suffix()
  qemu-img.c: Clean up handling of image size in img_create()

 cutils.c      |   17 ++++++++++++++---
 qemu-common.h |    7 +++++++
 qemu-img.c    |   23 +++++++++++++++++------
 3 files changed, 38 insertions(+), 9 deletions(-)

-- 
1.7.3.2

[Qemu-devel] [PATCH 1/2] Introduce strtosz_suffix()

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

This introduces strtosz_suffix() which allows the caller to specify a
default suffix in case the non default of MB is wanted.

strtosz() is kept as a wrapper for strtosz_suffix() which keeps it's
current default of MB.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 cutils.c      |   17 ++++++++++++++---
 qemu-common.h |    7 +++++++
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/cutils.c b/cutils.c
index 28089aa..1d24d9a 100644
--- a/cutils.c
+++ b/cutils.c
@@ -291,10 +291,10 @@ int fcntl_setfl(int fd, int flag)
  * value must be terminated by whitespace, ',' or '\0'. Return -1 on
  * error.
  */
-ssize_t strtosz(const char *nptr, char **end)
+ssize_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
 {
     ssize_t retval = -1;
-    char *endptr, c;
+    char *endptr, c, d;
     int mul_required = 0;
     double val, mul, integral, fraction;
 
@@ -313,10 +313,16 @@ ssize_t strtosz(const char *nptr, char **end)
      * part of a multi token argument.
      */
     c = *endptr;
+    d = c;
     if (isspace(c) || c == '\0' || c == ',') {
         c = 0;
+        if (default_suffix) {
+            d = default_suffix;
+        } else {
+            d = c;
+        }
     }
-    switch (c) {
+    switch (d) {
     case 'B':
     case 'b':
         mul = 1;
@@ -371,3 +377,8 @@ fail:
 
     return retval;
 }
+
+ssize_t strtosz(const char *nptr, char **end)
+{
+    return strtosz_suffix(nptr, end, 0);
+}
diff --git a/qemu-common.h b/qemu-common.h
index de82c2e..dc44cd6 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -149,7 +149,14 @@ time_t mktimegm(struct tm *tm);
 int qemu_fls(int i);
 int qemu_fdatasync(int fd);
 int fcntl_setfl(int fd, int flag);
+
+#define STRTOSZ_DEFSUFFIX_TB	'T'
+#define STRTOSZ_DEFSUFFIX_GB	'G'
+#define STRTOSZ_DEFSUFFIX_MB	'M'
+#define STRTOSZ_DEFSUFFIX_KB	'K'
+#define STRTOSZ_DEFSUFFIX_B	'B'
 ssize_t strtosz(const char *nptr, char **end);
+ssize_t strtosz_suffix(const char *nptr, char **end, const char);
 
 /* path.c */
 void init_paths(const char *prefix);
-- 
1.7.3.2

[Qemu-devel] [PATCH v5 0/2] Clean up img_create() and introduce strtosz_suffix()

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

This patch set introduces strtosz_suffix() which is needed to be able
to use strtosz parsing with a non MB default suffix. This is used to
clean up qemu-img.c:img_create().

Kevin asked me to rebase this instead of applying the other patches on
top, so please discard the previous versions. Sorry for the patch
noise.

v5 fixes the two issues pointed out by Stefan, making the call in
strtosz() explicitly use STRTOSZ_DEFSUFFIX_MB instead of 0 to specify
the default and adds a named argument to the prototype for
strtosz_suffix().

Jes Sorensen (2):
  Introduce strtosz_suffix()
  qemu-img.c: Clean up handling of image size in img_create()

 cutils.c      |   17 ++++++++++++++---
 qemu-common.h |    7 +++++++
 qemu-img.c    |   23 +++++++++++++++++------
 3 files changed, 38 insertions(+), 9 deletions(-)

-- 
1.7.3.2

[Qemu-devel] [PATCH 1/2] Introduce strtosz_suffix()

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

This introduces strtosz_suffix() which allows the caller to specify a
default suffix in case the non default of MB is wanted.

strtosz() is kept as a wrapper for strtosz_suffix() which keeps it's
current default of MB.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 cutils.c      |   17 ++++++++++++++---
 qemu-common.h |    7 +++++++
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/cutils.c b/cutils.c
index 28089aa..7984bc1 100644
--- a/cutils.c
+++ b/cutils.c
@@ -291,10 +291,10 @@ int fcntl_setfl(int fd, int flag)
  * value must be terminated by whitespace, ',' or '\0'. Return -1 on
  * error.
  */
-ssize_t strtosz(const char *nptr, char **end)
+ssize_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
 {
     ssize_t retval = -1;
-    char *endptr, c;
+    char *endptr, c, d;
     int mul_required = 0;
     double val, mul, integral, fraction;
 
@@ -313,10 +313,16 @@ ssize_t strtosz(const char *nptr, char **end)
      * part of a multi token argument.
      */
     c = *endptr;
+    d = c;
     if (isspace(c) || c == '\0' || c == ',') {
         c = 0;
+        if (default_suffix) {
+            d = default_suffix;
+        } else {
+            d = c;
+        }
     }
-    switch (c) {
+    switch (d) {
     case 'B':
     case 'b':
         mul = 1;
@@ -371,3 +377,8 @@ fail:
 
     return retval;
 }
+
+ssize_t strtosz(const char *nptr, char **end)
+{
+    return strtosz_suffix(nptr, end, STRTOSZ_DEFSUFFIX_MB);
+}
diff --git a/qemu-common.h b/qemu-common.h
index de82c2e..1ed32e5 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -149,7 +149,14 @@ time_t mktimegm(struct tm *tm);
 int qemu_fls(int i);
 int qemu_fdatasync(int fd);
 int fcntl_setfl(int fd, int flag);
+
+#define STRTOSZ_DEFSUFFIX_TB	'T'
+#define STRTOSZ_DEFSUFFIX_GB	'G'
+#define STRTOSZ_DEFSUFFIX_MB	'M'
+#define STRTOSZ_DEFSUFFIX_KB	'K'
+#define STRTOSZ_DEFSUFFIX_B	'B'
 ssize_t strtosz(const char *nptr, char **end);
+ssize_t strtosz_suffix(const char *nptr, char **end, const char default_suffix);
 
 /* path.c */
 void init_paths(const char *prefix);
-- 
1.7.3.2

Re: [Qemu-devel] [PATCH 1/2] Introduce strtosz_suffix()

[Markus Armbruster]

Sorry for the late review.

Jes.Sorensen@redhat.com writes:

> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> This introduces strtosz_suffix() which allows the caller to specify a
> default suffix in case the non default of MB is wanted.
>
> strtosz() is kept as a wrapper for strtosz_suffix() which keeps it's
> current default of MB.
>
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
>  cutils.c      |   17 ++++++++++++++---
>  qemu-common.h |    7 +++++++
>  2 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/cutils.c b/cutils.c
> index 28089aa..7984bc1 100644
> --- a/cutils.c
> +++ b/cutils.c
> @@ -291,10 +291,10 @@ int fcntl_setfl(int fd, int flag)
>   * value must be terminated by whitespace, ',' or '\0'. Return -1 on
>   * error.
>   */
> -ssize_t strtosz(const char *nptr, char **end)
> +ssize_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)

Last parameter's const is of marginal uility.  Matter of taste.

>  {
>      ssize_t retval = -1;
> -    char *endptr, c;
> +    char *endptr, c, d;
>      int mul_required = 0;
>      double val, mul, integral, fraction;
>  
> @@ -313,10 +313,16 @@ ssize_t strtosz(const char *nptr, char **end)
>       * part of a multi token argument.
>       */
>      c = *endptr;
> +    d = c;
>      if (isspace(c) || c == '\0' || c == ',') {
>          c = 0;
> +        if (default_suffix) {
> +            d = default_suffix;
> +        } else {
> +            d = c;
> +        }

The else clause "d = c" is effectively "d = 0" (due to prior "c = 0"),
which is the same as "d = default_suffix" (due to else's condition).
Thus, you can replace the conditional by a simple

    d = default_suffix;

>      }
> -    switch (c) {
> +    switch (d) {
>      case 'B':
>      case 'b':
>          mul = 1;
> @@ -371,3 +377,8 @@ fail:
>  
>      return retval;
>  }
> +
> +ssize_t strtosz(const char *nptr, char **end)
> +{
> +    return strtosz_suffix(nptr, end, STRTOSZ_DEFSUFFIX_MB);
> +}
> diff --git a/qemu-common.h b/qemu-common.h
> index de82c2e..1ed32e5 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -149,7 +149,14 @@ time_t mktimegm(struct tm *tm);
>  int qemu_fls(int i);
>  int qemu_fdatasync(int fd);
>  int fcntl_setfl(int fd, int flag);
> +
> +#define STRTOSZ_DEFSUFFIX_TB	'T'
> +#define STRTOSZ_DEFSUFFIX_GB	'G'
> +#define STRTOSZ_DEFSUFFIX_MB	'M'
> +#define STRTOSZ_DEFSUFFIX_KB	'K'
> +#define STRTOSZ_DEFSUFFIX_B	'B'

I don't see what these defines buy us over the literals, but if it makes
you or other reviewers happier...

>  ssize_t strtosz(const char *nptr, char **end);
> +ssize_t strtosz_suffix(const char *nptr, char **end, const char default_suffix);
>  
>  /* path.c */
>  void init_paths(const char *prefix);