Patchwork [v2,6/8] monitor: New argument type 'T'

login
register
mail settings
Submitter Markus Armbruster
Date Jan. 20, 2010, 4:08 p.m.
Message ID <1264003702-17329-7-git-send-email-armbru@redhat.com>
Download mbox | patch
Permalink /patch/43306/
State New
Headers show

Comments

Markus Armbruster - Jan. 20, 2010, 4:08 p.m.
This is a double value with optional suffixes ms, us, ns.  We'll need
this to get migrate_set_downtime() QMP-ready.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 monitor.c |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)
Luiz Capitulino - Jan. 21, 2010, 1:26 p.m.
On Wed, 20 Jan 2010 17:08:20 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> This is a double value with optional suffixes ms, us, ns.  We'll need
> this to get migrate_set_downtime() QMP-ready.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  monitor.c |   16 +++++++++++++++-
>  1 files changed, 15 insertions(+), 1 deletions(-)
> 
> diff --git a/monitor.c b/monitor.c
> index ce97e7b..6dafe0b 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -75,6 +75,9 @@
>   *              user mode accepts an optional G, g, M, m, K, k suffix,
>   *              which multiplies the value by 2^30 for suffixes G and
>   *              g, 2^20 for M and m, 2^10 for K and k
> + * 'T'          double
> + *              user mode accepts an optional ms, us, ns suffix,
> + *              which divides the value by 1e3, 1e6, 1e9, respectively
>   * '/'          optional gdb-like print format (like "/10x")
>   *
>   * '?'          optional type (for all types, except '/')
> @@ -3544,6 +3547,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>              }
>              break;
>          case 'b':
> +        case 'T':
>              {
>                  double val;
>  
> @@ -3558,7 +3562,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>                  if (get_double(mon, &val, &p) < 0) {
>                      goto fail;
>                  }
> -                if (*p) {
> +                if (c == 'b' && *p) {
>                      switch (*p) {
>                      case 'K': case 'k':
>                          val *= 1 << 10; p++; break;
> @@ -3568,6 +3572,16 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>                          val *= 1 << 30; p++; break;
>                      }
>                  }
> +                if (c == 'T' && p[0] && p[1] == 's') {

 Is this indexing of 'p' really correct? What if the value you're interested
is at the of the string? Like:

.args_type = "str:s,value:b"

> +                    switch (*p) {
> +                    case 'm':
> +                        val /= 1e3; p += 2; break;
> +                    case 'u':
> +                        val /= 1e6; p += 2; break;
> +                    case 'n':
> +                        val /= 1e9; p += 2; break;
> +                    }
> +                }
>                  if (*p && !qemu_isspace(*p)) {
>                      monitor_printf(mon, "Unknown unit suffix\n");
>                      goto fail;
Markus Armbruster - Jan. 21, 2010, 1:54 p.m.
Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Wed, 20 Jan 2010 17:08:20 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> This is a double value with optional suffixes ms, us, ns.  We'll need
>> this to get migrate_set_downtime() QMP-ready.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  monitor.c |   16 +++++++++++++++-
>>  1 files changed, 15 insertions(+), 1 deletions(-)
>> 
>> diff --git a/monitor.c b/monitor.c
>> index ce97e7b..6dafe0b 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -75,6 +75,9 @@
>>   *              user mode accepts an optional G, g, M, m, K, k suffix,
>>   *              which multiplies the value by 2^30 for suffixes G and
>>   *              g, 2^20 for M and m, 2^10 for K and k
>> + * 'T'          double
>> + *              user mode accepts an optional ms, us, ns suffix,
>> + *              which divides the value by 1e3, 1e6, 1e9, respectively
>>   * '/'          optional gdb-like print format (like "/10x")
>>   *
>>   * '?'          optional type (for all types, except '/')
>> @@ -3544,6 +3547,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>>              }
>>              break;
>>          case 'b':
>> +        case 'T':
>>              {
>>                  double val;
>>  
>> @@ -3558,7 +3562,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>>                  if (get_double(mon, &val, &p) < 0) {
>>                      goto fail;
>>                  }
>> -                if (*p) {
>> +                if (c == 'b' && *p) {
>>                      switch (*p) {
>>                      case 'K': case 'k':
>>                          val *= 1 << 10; p++; break;
>> @@ -3568,6 +3572,16 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>>                          val *= 1 << 30; p++; break;
>>                      }
>>                  }
>> +                if (c == 'T' && p[0] && p[1] == 's') {
>
>  Is this indexing of 'p' really correct? What if the value you're interested
> is at the of the string? Like:
>
> .args_type = "str:s,value:b"

p points right behind the floating-point number.  If no characters
follow, it points to the string's terminating zero.  If exactly one
character follows, it points to that one, and the next one is the
terminating zero.  If more than one follow, p points to the first one.

"p[0] && p[1] == 's'" is safe, because p[0] is safe, and if p[0] != 0,
then p[1] is also safe.

Convinced?

>> +                    switch (*p) {
>> +                    case 'm':
>> +                        val /= 1e3; p += 2; break;
>> +                    case 'u':
>> +                        val /= 1e6; p += 2; break;
>> +                    case 'n':
>> +                        val /= 1e9; p += 2; break;
>> +                    }
>> +                }
>>                  if (*p && !qemu_isspace(*p)) {
>>                      monitor_printf(mon, "Unknown unit suffix\n");
>>                      goto fail;
Luiz Capitulino - Jan. 21, 2010, 2:19 p.m.
On Thu, 21 Jan 2010 14:54:51 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > On Wed, 20 Jan 2010 17:08:20 +0100
> > Markus Armbruster <armbru@redhat.com> wrote:
> >
> >> This is a double value with optional suffixes ms, us, ns.  We'll need
> >> this to get migrate_set_downtime() QMP-ready.
> >> 
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >> ---
> >>  monitor.c |   16 +++++++++++++++-
> >>  1 files changed, 15 insertions(+), 1 deletions(-)
> >> 
> >> diff --git a/monitor.c b/monitor.c
> >> index ce97e7b..6dafe0b 100644
> >> --- a/monitor.c
> >> +++ b/monitor.c
> >> @@ -75,6 +75,9 @@
> >>   *              user mode accepts an optional G, g, M, m, K, k suffix,
> >>   *              which multiplies the value by 2^30 for suffixes G and
> >>   *              g, 2^20 for M and m, 2^10 for K and k
> >> + * 'T'          double
> >> + *              user mode accepts an optional ms, us, ns suffix,
> >> + *              which divides the value by 1e3, 1e6, 1e9, respectively
> >>   * '/'          optional gdb-like print format (like "/10x")
> >>   *
> >>   * '?'          optional type (for all types, except '/')
> >> @@ -3544,6 +3547,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
> >>              }
> >>              break;
> >>          case 'b':
> >> +        case 'T':
> >>              {
> >>                  double val;
> >>  
> >> @@ -3558,7 +3562,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
> >>                  if (get_double(mon, &val, &p) < 0) {
> >>                      goto fail;
> >>                  }
> >> -                if (*p) {
> >> +                if (c == 'b' && *p) {
> >>                      switch (*p) {
> >>                      case 'K': case 'k':
> >>                          val *= 1 << 10; p++; break;
> >> @@ -3568,6 +3572,16 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
> >>                          val *= 1 << 30; p++; break;
> >>                      }
> >>                  }
> >> +                if (c == 'T' && p[0] && p[1] == 's') {
> >
> >  Is this indexing of 'p' really correct? What if the value you're interested
> > is at the of the string? Like:
> >
> > .args_type = "str:s,value:b"
> 
> p points right behind the floating-point number.  If no characters
> follow, it points to the string's terminating zero.  If exactly one
> character follows, it points to that one, and the next one is the
> terminating zero.  If more than one follow, p points to the first one.
> 
> "p[0] && p[1] == 's'" is safe, because p[0] is safe, and if p[0] != 0,
> then p[1] is also safe.
> 
> Convinced?

 Yes, looks fine.

> 
> >> +                    switch (*p) {
> >> +                    case 'm':
> >> +                        val /= 1e3; p += 2; break;
> >> +                    case 'u':
> >> +                        val /= 1e6; p += 2; break;
> >> +                    case 'n':
> >> +                        val /= 1e9; p += 2; break;
> >> +                    }
> >> +                }
> >>                  if (*p && !qemu_isspace(*p)) {
> >>                      monitor_printf(mon, "Unknown unit suffix\n");
> >>                      goto fail;

Patch

diff --git a/monitor.c b/monitor.c
index ce97e7b..6dafe0b 100644
--- a/monitor.c
+++ b/monitor.c
@@ -75,6 +75,9 @@ 
  *              user mode accepts an optional G, g, M, m, K, k suffix,
  *              which multiplies the value by 2^30 for suffixes G and
  *              g, 2^20 for M and m, 2^10 for K and k
+ * 'T'          double
+ *              user mode accepts an optional ms, us, ns suffix,
+ *              which divides the value by 1e3, 1e6, 1e9, respectively
  * '/'          optional gdb-like print format (like "/10x")
  *
  * '?'          optional type (for all types, except '/')
@@ -3544,6 +3547,7 @@  static const mon_cmd_t *monitor_parse_command(Monitor *mon,
             }
             break;
         case 'b':
+        case 'T':
             {
                 double val;
 
@@ -3558,7 +3562,7 @@  static const mon_cmd_t *monitor_parse_command(Monitor *mon,
                 if (get_double(mon, &val, &p) < 0) {
                     goto fail;
                 }
-                if (*p) {
+                if (c == 'b' && *p) {
                     switch (*p) {
                     case 'K': case 'k':
                         val *= 1 << 10; p++; break;
@@ -3568,6 +3572,16 @@  static const mon_cmd_t *monitor_parse_command(Monitor *mon,
                         val *= 1 << 30; p++; break;
                     }
                 }
+                if (c == 'T' && p[0] && p[1] == 's') {
+                    switch (*p) {
+                    case 'm':
+                        val /= 1e3; p += 2; break;
+                    case 'u':
+                        val /= 1e6; p += 2; break;
+                    case 'n':
+                        val /= 1e9; p += 2; break;
+                    }
+                }
                 if (*p && !qemu_isspace(*p)) {
                     monitor_printf(mon, "Unknown unit suffix\n");
                     goto fail;