Re: [Qemu-devel] [PATCH 01/21] qemu-char: cleanup qmp_chardev_add

[Eric Blake]

On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> Use the usual idioms for error propagation.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  qemu-char.c | 56 +++++++++++++++++++++++++++++++-------------------------
>  1 file changed, 31 insertions(+), 25 deletions(-)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index 653ea10..f51c0aa 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -4214,6 +4214,7 @@ ChardevReturn *qmp_chardev_add(const char *id, 
> ChardevBackend *backend,
>  {
>      ChardevReturn *ret = g_new0(ChardevReturn, 1);
>      CharDriverState *base, *chr = NULL;
> +    Error *local_err = NULL;

It's a toss-up on whether 'local_err' or 'err' is the preferred name for
the local variable. I'm fine with either.


> @@ -4303,25 +4304,30 @@ ChardevReturn *qmp_chardev_add(const char *id, 
> ChardevBackend *backend,
>       * error then.
>       * TODO full conversion to Error API
>       */
> -    if (chr == NULL && errp && !*errp) {
> -        error_setg(errp, "Failed to create chardev");
> -    }
> -    if (chr) {
> -        chr->label = g_strdup(id);
> -        chr->avail_connections =
> -            (backend->kind == CHARDEV_BACKEND_KIND_MUX) ? MAX_MUX : 1;
> -        if (!chr->filename) {
> -            chr->filename = 
> g_strdup(ChardevBackendKind_lookup[backend->kind]);
> -        }
> -        if (!chr->explicit_be_open) {
> -            qemu_chr_be_event(chr, CHR_EVENT_OPENED);
> +    if (chr == NULL) {

I might write this as if (!chr), but that's cosmetic.

Conversion looks sane.
Reviewed-by: Eric Blake <address@hidden>

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

signature.asc
Description: OpenPGP digital signature