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

Submitted by Markus Armbruster on Jan. 20, 2010, 4:08 p.m.

Details

Message ID 1264003702-17329-7-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

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(-)

Comments

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 hide | download patch | download mbox

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;