Re: [Qemu-devel] [PATCH 03/22] qapi: add Error object

[Stefan Hajnoczi]

On Mon, Mar 7, 2011 at 1:22 AM, Anthony Liguori <address@hidden> wrote:
> +struct Error
> +{
> +    QDict *obj;
> +    const char *fmt;
> +    char *msg;
> +};

I wonder why fmt is const char * but msg is char *.  Users should use
error_get_pretty() instead of accessing msg directly and that function
returns const char * so it seems that msg should be const char * to
start with.

> +
> +void error_set(Error **errp, const char *fmt, ...)
> +{
> +    Error *err;
> +    va_list ap;
> +
> +    if (errp == NULL) {
> +        return;
> +    }
> +
> +    err = qemu_mallocz(sizeof(*err));
> +
> +    va_start(ap, fmt);
> +    err->obj = qobject_to_qdict(qobject_from_jsonv(fmt, &ap));

vsprintf() and friends pass va_list by value, they don't use a
pointer.  Perhaps you want to follow that idiom?

> +bool error_is_type(Error *err, const char *fmt)
> +{
> +    char *ptr;
> +    char *end;
> +    char classname[1024];
> +
> +    ptr = strstr(fmt, "'class': '");
> +    assert(ptr != NULL);
> +    ptr += strlen("'class': '");
> +
> +    end = strchr(ptr, '\'');
> +    assert(end != NULL);
> +
> +    memcpy(classname, ptr, (end - ptr));
> +    classname[(end - ptr)] = 0;
> +
> +    return strcmp(classname, error_get_field(err, "class")) == 0;

I'd get rid of the buffer/memcpy and use strncmp in-place instead:

const char *error_class = error_get_field(err, "class");
if (strlen(error_class) != end - ptr) {
    return false;
}
return strncmp(ptr, error_class, end - ptr) == 0;

Stefan