Re: [Qemu-devel] [PATCH] Drop superfluous conditionals around g_strdup()

[Fam Zheng]

On Thu, 12/04 10:26, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>  backends/rng-random.c    |  6 +-----
>  hw/tpm/tpm_passthrough.c |  4 +---
>  util/uri.c               | 43 +++++++++++++++++--------------------------
>  3 files changed, 19 insertions(+), 34 deletions(-)
> 
> diff --git a/backends/rng-random.c b/backends/rng-random.c
> index 601d9dc..4f85a8e 100644
> --- a/backends/rng-random.c
> +++ b/backends/rng-random.c
> @@ -88,11 +88,7 @@ static char *rng_random_get_filename(Object *obj, Error 
> **errp)
>  {
>      RndRandom *s = RNG_RANDOM(obj);
>  
> -    if (s->filename) {
> -        return g_strdup(s->filename);
> -    }
> -
> -    return NULL;
> +    return g_strdup(s->filename);
>  }
>  
>  static void rng_random_set_filename(Object *obj, const char *filename,
> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
> index 56e9e0f..2bf3c6f 100644
> --- a/hw/tpm/tpm_passthrough.c
> +++ b/hw/tpm/tpm_passthrough.c
> @@ -400,9 +400,7 @@ static int tpm_passthrough_handle_device_opts(QemuOpts 
> *opts, TPMBackend *tb)
>      const char *value;
>  
>      value = qemu_opt_get(opts, "cancel-path");
> -    if (value) {
> -        tb->cancel_path = g_strdup(value);
> -    }
> +    tb->cancel_path = g_strdup(value);
>  
>      value = qemu_opt_get(opts, "path");
>      if (!value) {
> diff --git a/util/uri.c b/util/uri.c
> index e348c17..bbf2832 100644
> --- a/util/uri.c
> +++ b/util/uri.c
> @@ -1736,24 +1736,21 @@ uri_resolve(const char *uri, const char *base) {
>       goto done;
>      if ((ref->scheme == NULL) && (ref->path == NULL) &&
>       ((ref->authority == NULL) && (ref->server == NULL))) {
> -     if (bas->scheme != NULL)
> -         res->scheme = g_strdup(bas->scheme);
> +        res->scheme = g_strdup(bas->scheme);
>       if (bas->authority != NULL)
>           res->authority = g_strdup(bas->authority);
>       else if (bas->server != NULL) {
> -         res->server = g_strdup(bas->server);
> -         if (bas->user != NULL)
> -             res->user = g_strdup(bas->user);
> -         res->port = bas->port;
> +            res->server = g_strdup(bas->server);
> +            res->user = g_strdup(bas->user);
> +            res->port = bas->port;
>       }
> -     if (bas->path != NULL)
> -         res->path = g_strdup(bas->path);
> -     if (ref->query != NULL)
> +        res->path = g_strdup(bas->path);
> +        if (ref->query != NULL) {
>           res->query = g_strdup (ref->query);
> -     else if (bas->query != NULL)
> -         res->query = g_strdup(bas->query);
> -     if (ref->fragment != NULL)
> -         res->fragment = g_strdup(ref->fragment);
> +        } else {
> +            res->query = g_strdup(bas->query);
> +        }
> +        res->fragment = g_strdup(ref->fragment);
>       goto step_7;
>      }
>  
> @@ -1767,13 +1764,10 @@ uri_resolve(const char *uri, const char *base) {
>       val = uri_to_string(ref);
>       goto done;
>      }
> -    if (bas->scheme != NULL)
> -     res->scheme = g_strdup(bas->scheme);
> +    res->scheme = g_strdup(bas->scheme);
>  
> -    if (ref->query != NULL)
> -     res->query = g_strdup(ref->query);
> -    if (ref->fragment != NULL)
> -     res->fragment = g_strdup(ref->fragment);
> +    res->query = g_strdup(ref->query);
> +    res->fragment = g_strdup(ref->fragment);
>  
>      /*
>       * 4) If the authority component is defined, then the reference is a
> @@ -1787,20 +1781,17 @@ uri_resolve(const char *uri, const char *base) {
>           res->authority = g_strdup(ref->authority);
>       else {
>           res->server = g_strdup(ref->server);
> -         if (ref->user != NULL)
> -             res->user = g_strdup(ref->user);
> +            res->user = g_strdup(ref->user);
>              res->port = ref->port;
>       }
> -     if (ref->path != NULL)
> -         res->path = g_strdup(ref->path);
> +        res->path = g_strdup(ref->path);
>       goto step_7;
>      }
>      if (bas->authority != NULL)
>       res->authority = g_strdup(bas->authority);
>      else if (bas->server != NULL) {
> -     res->server = g_strdup(bas->server);
> -     if (bas->user != NULL)
> -         res->user = g_strdup(bas->user);
> +        res->server = g_strdup(bas->server);
> +        res->user = g_strdup(bas->user);
>       res->port = bas->port;
>      }
>  
> -- 
> 1.9.3
> 
> 

Very confusing tab/whitespace mixture. Code is better, format is worse. I'm not
sure it's a win. :)

Fam