Re: [Qemu-trivial] [PATCH] util: Use g_malloc/g_free in envlist.c

[Laurent Vivier]

Please, cc: address@hidden

Laurent

Le 04/03/2017 à 11:53, Saurav Sachidanand a écrit :
> Change malloc/free to g_malloc/g_free in util/envlist.c, except for
> entry->env_var which is allocated using strdup(3).
> 
> Remove NULL checks for pointers returned from g_malloc as it exits
> in case of failure. Update calls to envlist_create to reflect this.
> 
> Free array returned by envlist_to_environ using g_free.
> 
> Update comments to reflect change in semantics.
> 
> Signed-off-by: Saurav Sachidanand <address@hidden>
> ---
>  bsd-user/main.c   | 12 +++---------
>  linux-user/main.c |  7 ++-----
>  util/envlist.c    | 31 +++++++++++++------------------
>  3 files changed, 18 insertions(+), 32 deletions(-)
> 
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index 714a692e6f..4d715840ff 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -744,10 +744,7 @@ int main(int argc, char **argv)
>      qemu_init_cpu_list();
>      module_call_init(MODULE_INIT_QOM);
>  
> -    if ((envlist = envlist_create()) == NULL) {
> -        (void) fprintf(stderr, "Unable to allocate envlist\n");
> -        exit(1);
> -    }
> +    envlist = envlist_create();
>  
>      /* add current environment into the list */
>      for (wrk = environ; *wrk != NULL; wrk++) {
> @@ -785,10 +782,7 @@ int main(int argc, char **argv)
>                  usage();
>          } else if (!strcmp(r, "ignore-environment")) {
>              envlist_free(envlist);
> -            if ((envlist = envlist_create()) == NULL) {
> -                (void) fprintf(stderr, "Unable to allocate envlist\n");
> -                exit(1);
> -            }
> +            envlist = envlist_create();
>          } else if (!strcmp(r, "U")) {
>              r = argv[optind++];
>              if (envlist_unsetenv(envlist, r) != 0)
> @@ -959,7 +953,7 @@ int main(int argc, char **argv)
>          free(*wrk);
>      }
>  
> -    free(target_environ);
> +    g_free(target_environ);
>  
>      if (qemu_loglevel_mask(CPU_LOG_PAGE)) {
>          qemu_log("guest_base  0x%lx\n", guest_base);
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 10a3bb3a12..3542c899f6 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -4229,10 +4229,7 @@ int main(int argc, char **argv, char **envp)
>      qemu_init_cpu_list();
>      module_call_init(MODULE_INIT_QOM);
>  
> -    if ((envlist = envlist_create()) == NULL) {
> -        (void) fprintf(stderr, "Unable to allocate envlist\n");
> -        exit(EXIT_FAILURE);
> -    }
> +    envlist = envlist_create();
>  
>      /* add current environment into the list */
>      for (wrk = environ; *wrk != NULL; wrk++) {
> @@ -4432,7 +4429,7 @@ int main(int argc, char **argv, char **envp)
>          free(*wrk);
>      }
>  
> -    free(target_environ);
> +    g_free(target_environ);
>  
>      if (qemu_loglevel_mask(CPU_LOG_PAGE)) {
>          qemu_log("guest_base  0x%lx\n", guest_base);
> diff --git a/util/envlist.c b/util/envlist.c
> index e86857e70a..bc21481b17 100644
> --- a/util/envlist.c
> +++ b/util/envlist.c
> @@ -17,16 +17,14 @@ static int envlist_parse(envlist_t *envlist,
>      const char *env, int (*)(envlist_t *, const char *));
>  
>  /*
> - * Allocates new envlist and returns pointer to that or
> - * NULL in case of error.
> + * Allocates new envlist and returns pointer to it.
>   */
>  envlist_t *
>  envlist_create(void)
>  {
>       envlist_t *envlist;
>  
> -     if ((envlist = malloc(sizeof (*envlist))) == NULL)
> -             return (NULL);
> +     envlist = g_malloc(sizeof (*envlist));
>  
>       QLIST_INIT(&envlist->el_entries);
>       envlist->el_count = 0;
> @@ -49,9 +47,9 @@ envlist_free(envlist_t *envlist)
>               QLIST_REMOVE(entry, ev_link);
>  
>               free((char *)entry->ev_var);
> -             free(entry);
> +             g_free(entry);
>       }
> -     free(envlist);
> +     g_free(envlist);
>  }
>  
>  /*
> @@ -156,15 +154,14 @@ envlist_setenv(envlist_t *envlist, const char *env)
>       if (entry != NULL) {
>               QLIST_REMOVE(entry, ev_link);
>               free((char *)entry->ev_var);
> -             free(entry);
> +             g_free(entry);
>       } else {
>               envlist->el_count++;
>       }
>  
> -     if ((entry = malloc(sizeof (*entry))) == NULL)
> -             return (errno);
> +     entry = g_malloc(sizeof (*entry));
>       if ((entry->ev_var = strdup(env)) == NULL) {
> -             free(entry);
> +             g_free(entry);
>               return (errno);
>       }
>       QLIST_INSERT_HEAD(&envlist->el_entries, entry, ev_link);
> @@ -202,7 +199,7 @@ envlist_unsetenv(envlist_t *envlist, const char *env)
>       if (entry != NULL) {
>               QLIST_REMOVE(entry, ev_link);
>               free((char *)entry->ev_var);
> -             free(entry);
> +             g_free(entry);
>  
>               envlist->el_count--;
>       }
> @@ -212,12 +209,12 @@ envlist_unsetenv(envlist_t *envlist, const char *env)
>  /*
>   * Returns given envlist as array of strings (in same form that
>   * global variable environ is).  Caller must free returned memory
> - * by calling free(3) for each element and for the array.  Returned
> - * array and given envlist are not related (no common references).
> + * by calling free(3) for each element, and g_free for the array.
> + * Returned array and given envlist are not related (no common
> + * references).
>   *
>   * If caller provides count pointer, number of items in array is
> - * stored there.  In case of error, NULL is returned and no memory
> - * is allocated.
> + * stored there.
>   */
>  char **
>  envlist_to_environ(const envlist_t *envlist, size_t *count)
> @@ -225,9 +222,7 @@ envlist_to_environ(const envlist_t *envlist, size_t 
> *count)
>       struct envlist_entry *entry;
>       char **env, **penv;
>  
> -     penv = env = malloc((envlist->el_count + 1) * sizeof (char *));
> -     if (env == NULL)
> -             return (NULL);
> +     penv = env = g_malloc((envlist->el_count + 1) * sizeof (char *));
>  
>       for (entry = envlist->el_entries.lh_first; entry != NULL;
>           entry = entry->ev_link.le_next) {
>