[Qemu-devel] Re: [PATCH 2/7] Introduce QError

[Anthony Liguori <aliguori@us.ibm.com>]

> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  Makefile  |    2 +-
>  qerror.c  |  240 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qerror.h  |   48 ++++++++++++
>  qobject.h |    1 +
>  4 files changed, 290 insertions(+), 1 deletions(-)
>  create mode 100644 qerror.c
>  create mode 100644 qerror.h
>
> diff --git a/Makefile b/Makefile
> index 813bd0a..325e583 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -125,7 +125,7 @@ obj-y += net.o net-queue.o
>  obj-y += qemu-char.o aio.o net-checksum.o savevm.o
>  obj-y += msmouse.o ps2.o
>  obj-y += qdev.o qdev-properties.o
> -obj-y += qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o qjson.o
> +obj-y += qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o qjson.o qerror.o
>  obj-y += qemu-config.o
>
>  obj-$(CONFIG_BRLAPI) += baum.o
> diff --git a/qerror.c b/qerror.c
> new file mode 100644
> index 0000000..0359d65
> --- /dev/null
> +++ b/qerror.c
> @@ -0,0 +1,240 @@
> +/*
> + * QError: QEMU Error data-type.
> + *
> + * Copyright (C) 2009 Red Hat Inc.
> + *
> + * Authors:
> + *  Luiz Capitulino <lcapitulino@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + */
> +#include "qint.h"
> +#include "qjson.h"
> +#include "qerror.h"
> +#include "qstring.h"
> +#include "sysemu.h"
> +#include "qemu-common.h"
> +
> +static void qerror_destroy_obj(QObject *obj);
> +
> +static const QType qerror_type = {
> +    .code = QTYPE_QERROR,
> +    .destroy = qerror_destroy_obj,
> +};
> +
> +/**
> + * The 'desc' member is a printf-like string, the format of the format
> + * string is:
> + *
> + * %(KEY)TYPE
> + *
> + * Where KEY is a QDict key and TYPE is the type of its value, KEY and
> + * its value must be passed to qerror_from_info().
> + *
> + * Example:
> + *
> + * "foo error on device: %(device)s slot: %(slot_nr)s"
> + *
> + * A single percent sign can be printed if followed by a second one,
> + * for example:
> + *
> + * "running out of foo: %(foo)d%%"
> + *
> + * Valid types are:
> + *
> + * s    (string)
> + * d    (integer)
> + */
> +static QErrorTable qerror_table[] = {
> +    {
> +        .code = QERR_UNKNOWN,
> +        .desc = "unknown error",
> +    },
> +};
> +
> +/**
> + * qerror_new(): Create a new QError
> + *
> + * Return strong reference.
> + */
> +QError *qerror_new(void)
> +{
> +    QError *qerr;
> +
> +    qerr = qemu_mallocz(sizeof(*qerr));
> +    QOBJECT_INIT(qerr, &qerror_type);
> +
> +    return qerr;
> +}
> +
> +/**
> + * qerror_from_info(): Create a new QError from error information
> + *
> + * The information consists of:
> + *
> + * - code: error code
> + * - file: the file of where the error happend
> + * - linenr: the line number of where the error happend
> + * - fmt: JSON printf-like format
> + * - va: va_list of all arguments
> + *
> + * Return strong reference.
> + */
> +QError *qerror_from_info(QErrorCode code, const char *file, int linenr,
> +                         const char *fmt, va_list *va)
> +{
>   

va_list doesn't need to be a pointer.

Shouldn't this take a ... too?

> diff --git a/qerror.h b/qerror.h
> new file mode 100644
> index 0000000..2fd0d58
> --- /dev/null
> +++ b/qerror.h
> @@ -0,0 +1,48 @@
> +/*
> + * QError header file.
> + *
> + * Copyright (C) 2009 Red Hat Inc.
> + *
> + * Authors:
> + *  Luiz Capitulino <lcapitulino@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + */
> +#ifndef QERROR_H
> +#define QERROR_H
> +
> +#include <stdarg.h>
> +#include "qobject.h"
> +
> +/*
> + * IMPORTANT: errors numbers must not change after they have been
> + * added here.
> + */
> +typedef enum QErrorCode {
> +    QERR_UNKNOWN,
> +    QERR_MAX,
> +} QErrorCode;
> +
> +struct QError;
> +
> +typedef struct QErrorTable {
> +    QErrorCode code;
> +    const char *desc;
> +} QErrorTable;
> +
> +typedef struct QError {
> +    QObject_HEAD;
> +    int linenr;         /* error line number */
> +    const char *file;   /* error file */
> +    QObject *data;      /* error specific data */
> +    const QErrorTable *entry;
> +} QError;
> +
> +QError *qerror_new(void);
> +QError *qerror_from_info(QErrorCode code, const char *file, int linenr,
> +                         const char *fmt, va_list *va);
>   

I don't know how I feel about the linenr/file bit.  It seems prone to 
misused when used in a public interface (vs. automatically via a macro).

-- 
Regards,

Anthony Liguori