Re: [Qemu-devel] [PATCH v2 12/26] qapi: Improve qobject input visitor error reporting

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2513 bytes --]

On 02/26/2017 03:43 PM, Markus Armbruster wrote:
> Error messages refer to nodes of the QObject being visited by name.
> Trouble is the names are sometimes less than helpful:
> 

> Improve error messages by referring to nodes by path instead, as
> follows:
> 
> * The path of the root QObject is whatever @name argument got passed
>   to the visitor, except NULL gets mapped to "<anonymous>".
> 
> * The path of a root QDict's member is the member key.
> 
> * The path of a root QList's member is "[%zu]", where %zu is the list
>   index, starting at zero.
> 
> * The path of a non-root QDict's member is the path of the QDict
>   concatenated with "." and the member key.
> 
> * The path of a non-root QList's member is the path of the QList
>   concatenated with "[%zu]", where %zu is the list index.

%zu here, but...


> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/qapi/visitor.h       |   6 ---
>  qapi/qobject-input-visitor.c | 121 +++++++++++++++++++++++++++++++------------
>  2 files changed, 87 insertions(+), 40 deletions(-)
> 

>  
> -typedef struct StackObject
> -{
> -    QObject *obj; /* Object being visited */
> +typedef struct StackObject {
> +    const char *name;            /* Name of @obj in its parent, if any */
> +    QObject *obj;                /* QDict or QList being visited */
>      void *qapi; /* sanity check that caller uses same pointer */
>  
> -    GHashTable *h;           /* If obj is dict: unvisited keys */
> -    const QListEntry *entry; /* If obj is list: unvisited tail */
> +    GHashTable *h;              /* If @obj is QDict: unvisited keys */
> +    const QListEntry *entry;    /* If @obj is QList: unvisited tail */
> +    unsigned index;             /* If @obj is QList: list index of @entry */

> +    QSLIST_FOREACH(so , &qiv->stack, node) {
> +        if (qobject_type(so->obj) == QTYPE_QDICT) {
> +            g_string_prepend(qiv->errname, name);
> +            g_string_prepend_c(qiv->errname, '.');
> +        } else {
> +            snprintf(buf, sizeof(buf), "[%u]", so->index);

...%u here.  Minor inconsistency in the commit message, but I'm okay
mere 'unsigned' in the code, as we'd probably have other problems long
before hitting a list with 2 billion entries, even on platforms where
size_t is 64-bit.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]