Patchwork [06/19] monitor: do_info_cpus(): Use QBool

login
register
mail settings
Submitter Luiz Capitulino
Date Dec. 9, 2009, 4:27 p.m.
Message ID <1260376078-8694-7-git-send-email-lcapitulino@redhat.com>
Download mbox | patch
Permalink /patch/40741/
State New
Headers show

Comments

Luiz Capitulino - Dec. 9, 2009, 4:27 p.m.
While there update the documentation as well.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |   39 ++++++++++++++++++++++++++-------------
 1 files changed, 26 insertions(+), 13 deletions(-)
Markus Armbruster - Dec. 10, 2009, 10:44 a.m.
Luiz Capitulino <lcapitulino@redhat.com> writes:

> While there update the documentation as well.
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  monitor.c |   39 ++++++++++++++++++++++++++-------------
>  1 files changed, 26 insertions(+), 13 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index aa56ec7..8729535 100644
> --- a/monitor.c
> +++ b/monitor.c
[...]
> @@ -650,12 +652,22 @@ static void monitor_print_cpus(Monitor *mon, const QObject *data)
>  /**
>   * do_info_cpus(): Show CPU information
>   *
> - * Return a QList with a QDict for each CPU.
> + * Return a QList. Each CPU is represented by a QDict, which contains:
>   *
> - * For example:
> + * - "cpu": CPU index
> + * - "current": true if this is the current CPU, false otherwise
> + * - "halted": true if the cpu is halted, false otherwise
> + * - Current program counter, in decimal. The key name depends on

Do we want to specify the base for numbers in the JSON?

If yes, why not use decimal everywhere?

Aside, if we use hexadecimal in JSON at all, then I'd prefer addresses
to be hexadecimal.

> + *   the architecture:
> + *      "pc": i386/x86)64
> + *      "nip": PPC
> + *      "pc" and "npc": sparc
> + *      "PC": mips
>   *
> - * [ { "CPU": 0, "current": "yes", "pc": 0x..., "halted": "no" },
> - *   { "CPU": 1, "current": "no",  "pc": 0x..., "halted": "yes" } ]
> + * Example:
> + *
> + * [ { "CPU": 0, "current": true, "halted": false, "pc": 3227107138 },
> + *   { "CPU": 1, "current": false, "halted": true, "pc": 7108165 } ]
>   */
>  static void do_info_cpus(Monitor *mon, QObject **ret_data)
[...]
Luiz Capitulino - Dec. 10, 2009, noon
On Thu, 10 Dec 2009 11:44:49 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > While there update the documentation as well.
> >
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> >  monitor.c |   39 ++++++++++++++++++++++++++-------------
> >  1 files changed, 26 insertions(+), 13 deletions(-)
> >
> > diff --git a/monitor.c b/monitor.c
> > index aa56ec7..8729535 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> [...]
> > @@ -650,12 +652,22 @@ static void monitor_print_cpus(Monitor *mon, const QObject *data)
> >  /**
> >   * do_info_cpus(): Show CPU information
> >   *
> > - * Return a QList with a QDict for each CPU.
> > + * Return a QList. Each CPU is represented by a QDict, which contains:
> >   *
> > - * For example:
> > + * - "cpu": CPU index
> > + * - "current": true if this is the current CPU, false otherwise
> > + * - "halted": true if the cpu is halted, false otherwise
> > + * - Current program counter, in decimal. The key name depends on
> 
> Do we want to specify the base for numbers in the JSON?
> 
> If yes, why not use decimal everywhere?
> 
> Aside, if we use hexadecimal in JSON at all, then I'd prefer addresses
> to be hexadecimal.

 Makes sense, I was in doubt about how I should represent this but addresses
should always be hexadecimal.

 What about adding a section to do the spec (or creating a new
document) describing the protocol style?
Anthony Liguori - Dec. 10, 2009, 1:01 p.m.
Markus Armbruster wrote:
> Do we want to specify the base for numbers in the JSON?
>
> If yes, why not use decimal everywhere?
>
> Aside, if we use hexadecimal in JSON at all, then I'd prefer addresses
> to be hexadecimal.
>   

Hexadecimal is not valid to represent numbers in JSON.
Luiz Capitulino - Dec. 10, 2009, 1:05 p.m.
On Thu, 10 Dec 2009 07:01:22 -0600
Anthony Liguori <aliguori@linux.vnet.ibm.com> wrote:

> Markus Armbruster wrote:
> > Do we want to specify the base for numbers in the JSON?
> >
> > If yes, why not use decimal everywhere?
> >
> > Aside, if we use hexadecimal in JSON at all, then I'd prefer addresses
> > to be hexadecimal.
> >   
> 
> Hexadecimal is not valid to represent numbers in JSON.

 We can return a string.
Anthony Liguori - Dec. 10, 2009, 1:08 p.m.
Luiz Capitulino wrote:
> On Thu, 10 Dec 2009 07:01:22 -0600
> Anthony Liguori <aliguori@linux.vnet.ibm.com> wrote:
>
>   
>> Markus Armbruster wrote:
>>     
>>> Do we want to specify the base for numbers in the JSON?
>>>
>>> If yes, why not use decimal everywhere?
>>>
>>> Aside, if we use hexadecimal in JSON at all, then I'd prefer addresses
>>> to be hexadecimal.
>>>   
>>>       
>> Hexadecimal is not valid to represent numbers in JSON.
>>     
>
>  We can return a string.
>   
Returning a hex string is a bad idea.  It overcomplicates clients.
Anthony Liguori - Dec. 10, 2009, 1:10 p.m.
Luiz Capitulino wrote:
> On Thu, 10 Dec 2009 07:01:22 -0600
> Anthony Liguori <aliguori@linux.vnet.ibm.com> wrote:
>
>   
>> Markus Armbruster wrote:
>>     
>>> Do we want to specify the base for numbers in the JSON?
>>>
>>> If yes, why not use decimal everywhere?
>>>
>>> Aside, if we use hexadecimal in JSON at all, then I'd prefer addresses
>>> to be hexadecimal.
>>>   
>>>       
>> Hexadecimal is not valid to represent numbers in JSON.
>>     
>
>  We can return a string.
>   

When it comes to returning u64s, we'll have to send these over the wire 
as s64s.  It's not ideal but it's the only way to support languages that 
don't distinguish between signed and unsigned types.
Markus Armbruster - Dec. 10, 2009, 1:22 p.m.
Anthony Liguori <aliguori@linux.vnet.ibm.com> writes:

> Markus Armbruster wrote:
>> Do we want to specify the base for numbers in the JSON?
>>
>> If yes, why not use decimal everywhere?
>>
>> Aside, if we use hexadecimal in JSON at all, then I'd prefer addresses
>> to be hexadecimal.
>>   
>
> Hexadecimal is not valid to represent numbers in JSON.

In that case, there's no need to document the format of numbers for
every command.

Patch

diff --git a/monitor.c b/monitor.c
index aa56ec7..8729535 100644
--- a/monitor.c
+++ b/monitor.c
@@ -611,8 +611,9 @@  static void print_cpu_iter(QObject *obj, void *opaque)
     assert(qobject_type(obj) == QTYPE_QDICT);
     cpu = qobject_to_qdict(obj);
 
-    if (strcmp(qdict_get_str(cpu, "current"), "yes") == 0)
+    if (qdict_get_bool(cpu, "current")) {
         active = '*';
+    }
 
     monitor_printf(mon, "%c CPU #%d: ", active, (int)qdict_get_int(cpu, "CPU"));
 
@@ -632,8 +633,9 @@  static void print_cpu_iter(QObject *obj, void *opaque)
                    (target_long) qdict_get_int(cpu, "PC"));
 #endif
 
-    if (strcmp(qdict_get_str(cpu, "halted"), "yes") == 0)
+    if (qdict_get_bool(cpu, "halted")) {
         monitor_printf(mon, " (halted)");
+    }
 
     monitor_printf(mon, "\n");
 }
@@ -650,12 +652,22 @@  static void monitor_print_cpus(Monitor *mon, const QObject *data)
 /**
  * do_info_cpus(): Show CPU information
  *
- * Return a QList with a QDict for each CPU.
+ * Return a QList. Each CPU is represented by a QDict, which contains:
  *
- * For example:
+ * - "cpu": CPU index
+ * - "current": true if this is the current CPU, false otherwise
+ * - "halted": true if the cpu is halted, false otherwise
+ * - Current program counter, in decimal. The key name depends on
+ *   the architecture:
+ *      "pc": i386/x86)64
+ *      "nip": PPC
+ *      "pc" and "npc": sparc
+ *      "PC": mips
  *
- * [ { "CPU": 0, "current": "yes", "pc": 0x..., "halted": "no" },
- *   { "CPU": 1, "current": "no",  "pc": 0x..., "halted": "yes" } ]
+ * Example:
+ *
+ * [ { "CPU": 0, "current": true, "halted": false, "pc": 3227107138 },
+ *   { "CPU": 1, "current": false, "halted": true, "pc": 7108165 } ]
  */
 static void do_info_cpus(Monitor *mon, QObject **ret_data)
 {
@@ -668,14 +680,17 @@  static void do_info_cpus(Monitor *mon, QObject **ret_data)
     mon_get_cpu();
 
     for(env = first_cpu; env != NULL; env = env->next_cpu) {
-        const char *answer;
-        QDict *cpu = qdict_new();
+        QDict *cpu;
+        QObject *obj;
 
         cpu_synchronize_state(env);
 
-        qdict_put(cpu, "CPU", qint_from_int(env->cpu_index));
-        answer = (env == mon->mon_cpu) ? "yes" : "no";
-        qdict_put(cpu, "current", qstring_from_str(answer));
+        obj = qobject_from_jsonf("{ 'CPU': %d, 'current': %i, 'halted': %i }",
+                                 env->cpu_index, env == mon->mon_cpu,
+                                 env->halted);
+        assert(obj != NULL);
+
+        cpu = qobject_to_qdict(obj);
 
 #if defined(TARGET_I386)
         qdict_put(cpu, "pc", qint_from_int(env->eip + env->segs[R_CS].base));
@@ -687,8 +702,6 @@  static void do_info_cpus(Monitor *mon, QObject **ret_data)
 #elif defined(TARGET_MIPS)
         qdict_put(cpu, "PC", qint_from_int(env->active_tc.PC));
 #endif
-        answer = env->halted ? "yes" : "no";
-        qdict_put(cpu, "halted", qstring_from_str(answer));
 
         qlist_append(cpu_list, cpu);
     }