Re: [Qemu-devel] [PATCH 08/14] qerror: add build_error_dict() and error_object_table[]

[Markus Armbruster]

Luiz Capitulino <address@hidden> writes:

> In the near future, the QERR_ macros (which are json strings today) will
> be turned into an enumeration. When we get there, build_error_dict()
> will be used to (guess what) build an error dict by:
>
>  1. Using the error class as an index to error_object_table[], which
>     contains all QMP errors as json strings (with default values)
>
>  2. Use the human error string to construct the error object data member.
>     For example, an error message like:
>
>      "Parameter name=brain has not been found"
>
>      Will construct the following data member:
>
>      'data': { 'name': 'brain' } }
>
> Signed-off-by: Luiz Capitulino <address@hidden>
> ---
>  qerror.c               | 86 
> ++++++++++++++++++++++++++++++++++++++++++++++++++
>  qerror.h               |  1 +
>  scripts/qapi-errors.py | 39 +++++++++++++++++++++++
>  3 files changed, 126 insertions(+)
>
> diff --git a/qerror.c b/qerror.c
> index 42e8687..267545e 100644
> --- a/qerror.c
> +++ b/qerror.c
> @@ -38,6 +38,92 @@ static QError *qerror_new(void)
>      return qerr;
>  }
>  
> +static bool iscchar(int c)
> +{
> +    return (isalpha(c) || isdigit(c) || c == '_');
> +}

Depends on locale, which may not be what you want.

Beware of passing negative arguments to ctype.h functions.

> +
> +static char *get_key(const char *str)
> +{
> +    char *p, *ret;
> +
> +    ret = p = g_strdup(str);
> +
> +    while (!iscchar(*p)) {
> +        p++;
> +    }
> +    memmove(ret, p, strlen(ret));
> +
> +    p = ret;
> +    while (iscchar(*p)) {
> +        p++;
> +    }
> +    *p = '\0';
> +
> +    return ret;
> +}

Suggest something like

static char *get_key(const char *str)
{
    char *beg, *end;

    for (beg = str; !iscchar(*beg); beg++) ;
    for (end = beg; iscchar(*end); end++) ;
    return g_strndup(beg, end - beg);
}

> +
> +static char *get_value(const char *str)
> +{
> +    char *p, *ret;
> +
> +    p = strchr(str, '=');
> +    while (!iscchar(*p)) {
> +        p++;
> +    }
> +    p = ret = g_strdup(p);
> +    while (iscchar(*p)) {
> +        p++;
> +    }
> +    *p = '\0';
> +
> +    return ret;
> +}

Likewise.

> +
> +static void set_dict_data(const char *msg, QDict *data_dict)
> +{
> +    char *str, *msg2, *saveptr = NULL;
> +
> +    msg2 = g_strdup(msg);
> +    str = strtok_r(msg2, " ", &saveptr);
> +    while (str) {
> +        if (strchr(str, '=')) {
> +            char *key = get_key(str);
> +            char *value = get_value(str);
> +
> +            /* FIXME: handle ints */
> +            if (qdict_haskey(data_dict, key)) {
> +                qdict_put(data_dict, key, qstring_from_str(value));
> +            }
> +
> +            g_free(key);
> +            g_free(value);
> +        }
> +        str = strtok_r(NULL, " ", &saveptr);
> +    }
> +
> +    g_free(msg2);
> +}
> +
> +QDict *build_error_dict(int err_class, const char *msg)
> +{
> +    QDict *err_dict;
> +    QObject *obj;
> +
> +    assert(msg[0] != '\0');
> +
> +    obj = qobject_from_json(error_object_table[err_class]);
> +    assert(obj);
> +    assert(qobject_type(obj) == QTYPE_QDICT);
> +
> +    err_dict = qobject_to_qdict(obj);
> +    assert(qdict_haskey(err_dict, "data"));
> +
> +    set_dict_data(msg, qdict_get_qdict(err_dict, "data"));
> +
> +    return err_dict;
> +}
> +
>  static QDict *error_object_from_fmt(const char *fmt, va_list *va)
>  {
>      QObject *obj;
> diff --git a/qerror.h b/qerror.h
> index 16401ff..c4f6053 100644
> --- a/qerror.h
> +++ b/qerror.h
> @@ -36,5 +36,6 @@ void qerror_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
>  void qerror_report_err(Error *err);
>  void assert_no_error(Error *err);
>  char *qerror_format(const char *fmt, QDict *error);
> +QDict *build_error_dict(int err_class, const char *msg);
>  
>  #endif /* QERROR_H */
> diff --git a/scripts/qapi-errors.py b/scripts/qapi-errors.py
> index 59cf426..5f8723e 100644
> --- a/scripts/qapi-errors.py
> +++ b/scripts/qapi-errors.py
> @@ -85,6 +85,42 @@ static const QErrorStringTable qerror_table[] = {
>  
>      return ret
>  
> +def gen_error_data_obj(data):
> +    colon = ''
> +    data_str = ''
> +    for k, v in data.items():
> +        data_str += colon + "'%s': " % k
> +        if v == 'str':
> +            data_str += "'unknown'"
> +        elif v == 'int':
> +            data_str += '0'
> +        else:
> +            sys.exit("unknown data type '%s' for error '%s'" % (v, name))
> +        colon = ', '
> +    return data_str

colon is either empty or ', ', but never a colon.  What about calling it
sep, for separator?

> +
> +def gen_error_obj_table(exprs):
> +    ret = mcgen('''
> +static const char *error_object_table[] = {
> +''')
> +
> +    for err in exprs:
> +        data = gen_error_data_obj({})
> +        if err.has_key('data'):
> +            data = gen_error_data_obj(err['data'])
> +        ret += mcgen('''
> +    "{ 'class': '%(error_class)s', 'data': { %(error_data)s } }",
> +''',
> +                error_class=err['error'], error_data=data)
> +
> +    ret += mcgen('''
> +    NULL,
> +};
> +
> +''')
> +
> +    return ret;
> +
>  def gen_error_macro_data_str(data):
>      colon = ''
>      data_str = ''
> @@ -173,5 +209,8 @@ if __name__ == '__main__':
>      ret = gen_error_def_table(exprs)
>      fdef.write(ret)
>  
> +    ret = gen_error_obj_table(exprs)
> +    fdef.write(ret)
> +
>      fdef.flush()
>      fdef.close()