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

Saurav Sachidanand posted 1 patch 7 years ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170320173828.3874-1-sauravsachidanand@gmail.com
Test checkpatch failed
Test docker passed
Test s390x passed
bsd-user/main.c   | 14 ++++----------
linux-user/main.c |  9 +++------
util/envlist.c    | 47 +++++++++++++++++++----------------------------
3 files changed, 26 insertions(+), 44 deletions(-)
[Qemu-devel] [Qemu-trivial] [PATCH v5] util: Use g_malloc/g_free in envlist.c
Posted by Saurav Sachidanand 7 years ago
Change malloc/strdup/free to g_malloc/g_strdup/g_free in
util/envlist.c.

Remove NULL checks for pointers returned from g_malloc and g_strdup
as they exit in case of failure. Also, update calls to envlist_create
to reflect this.

Free array and array contents returned by envlist_to_environ using
g_free in bsd-user/main.c and linux-user/main.c.

Update comments to reflect change in semantics.

Signed-off-by: Saurav Sachidanand <sauravsachidanand@gmail.com>
---
 bsd-user/main.c   | 14 ++++----------
 linux-user/main.c |  9 +++------
 util/envlist.c    | 47 +++++++++++++++++++----------------------------
 3 files changed, 26 insertions(+), 44 deletions(-)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index 714a692e6f..04f95ddd54 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)
@@ -956,10 +950,10 @@ int main(int argc, char **argv)
     }
 
     for (wrk = target_environ; *wrk; wrk++) {
-        free(*wrk);
+        g_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..5f20769cb9 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++) {
@@ -4429,10 +4426,10 @@ int main(int argc, char **argv, char **envp)
     }
 
     for (wrk = target_environ; *wrk; wrk++) {
-        free(*wrk);
+        g_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..1eeb7fca87 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;
@@ -48,10 +46,10 @@ envlist_free(envlist_t *envlist)
 		entry = envlist->el_entries.lh_first;
 		QLIST_REMOVE(entry, ev_link);
 
-		free((char *)entry->ev_var);
-		free(entry);
+		g_free((char *)entry->ev_var);
+		g_free(entry);
 	}
-	free(envlist);
+	g_free(envlist);
 }
 
 /*
@@ -101,8 +99,7 @@ envlist_parse(envlist_t *envlist, const char *env,
 	if ((envlist == NULL) || (env == NULL))
 		return (EINVAL);
 
-	if ((tmpenv = strdup(env)) == NULL)
-		return (errno);
+	tmpenv = g_strdup(env);
     envsave = tmpenv;
 
     do {
@@ -117,7 +114,7 @@ envlist_parse(envlist_t *envlist, const char *env,
         tmpenv = envvar + 1;
     } while (envvar != NULL);
 
-    free(envsave);
+    g_free(envsave);
     return ret;
 }
 
@@ -155,18 +152,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((char *)entry->ev_var);
+		g_free(entry);
 	} else {
 		envlist->el_count++;
 	}
 
-	if ((entry = malloc(sizeof (*entry))) == NULL)
-		return (errno);
-	if ((entry->ev_var = strdup(env)) == NULL) {
-		free(entry);
-		return (errno);
-	}
+	entry = g_malloc(sizeof(*entry));
+	entry->ev_var = g_strdup(env);
 	QLIST_INSERT_HEAD(&envlist->el_entries, entry, ev_link);
 
 	return (0);
@@ -201,8 +194,8 @@ 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((char *)entry->ev_var);
+		g_free(entry);
 
 		envlist->el_count--;
 	}
@@ -212,12 +205,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 g_free for each element and 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,13 +218,11 @@ 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) {
-		*(penv++) = strdup(entry->ev_var);
+		*(penv++) = g_strdup(entry->ev_var);
 	}
 	*penv = NULL; /* NULL terminate the list */
 
-- 
2.12.0


Re: [Qemu-devel] [Qemu-trivial] [PATCH v5] util: Use g_malloc/g_free in envlist.c
Posted by Stefan Hajnoczi 7 years ago
On Mon, Mar 20, 2017 at 05:38:28PM +0000, Saurav Sachidanand wrote:
> Change malloc/strdup/free to g_malloc/g_strdup/g_free in
> util/envlist.c.
> 
> Remove NULL checks for pointers returned from g_malloc and g_strdup
> as they exit in case of failure. Also, update calls to envlist_create
> to reflect this.
> 
> Free array and array contents returned by envlist_to_environ using
> g_free in bsd-user/main.c and linux-user/main.c.
> 
> Update comments to reflect change in semantics.
> 
> Signed-off-by: Saurav Sachidanand <sauravsachidanand@gmail.com>
> ---
>  bsd-user/main.c   | 14 ++++----------
>  linux-user/main.c |  9 +++------
>  util/envlist.c    | 47 +++++++++++++++++++----------------------------
>  3 files changed, 26 insertions(+), 44 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Re: [Qemu-devel] [Qemu-trivial] [PATCH v5] util: Use g_malloc/g_free in envlist.c
Posted by no-reply@patchew.org 7 years ago
Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [Qemu-trivial] [PATCH v5] util: Use g_malloc/g_free in envlist.c
Message-id: 20170320173828.3874-1-sauravsachidanand@gmail.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
2fe2b02 util: Use g_malloc/g_free in envlist.c

=== OUTPUT BEGIN ===
Checking PATCH 1/1: util: Use g_malloc/g_free in envlist.c...
ERROR: code indent should never use tabs
#111: FILE: util/envlist.c:27:
+^Ienvlist = g_malloc(sizeof(*envlist));$

ERROR: code indent should never use tabs
#121: FILE: util/envlist.c:49:
+^I^Ig_free((char *)entry->ev_var);$

ERROR: code indent should never use tabs
#122: FILE: util/envlist.c:50:
+^I^Ig_free(entry);$

ERROR: code indent should never use tabs
#125: FILE: util/envlist.c:52:
+^Ig_free(envlist);$

ERROR: code indent should never use tabs
#135: FILE: util/envlist.c:102:
+^Itmpenv = g_strdup(env);$

ERROR: code indent should never use tabs
#154: FILE: util/envlist.c:155:
+^I^Ig_free((char *)entry->ev_var);$

ERROR: code indent should never use tabs
#155: FILE: util/envlist.c:156:
+^I^Ig_free(entry);$

ERROR: code indent should never use tabs
#166: FILE: util/envlist.c:161:
+^Ientry = g_malloc(sizeof(*entry));$

ERROR: code indent should never use tabs
#167: FILE: util/envlist.c:162:
+^Ientry->ev_var = g_strdup(env);$

ERROR: code indent should never use tabs
#177: FILE: util/envlist.c:197:
+^I^Ig_free((char *)entry->ev_var);$

ERROR: code indent should never use tabs
#178: FILE: util/envlist.c:198:
+^I^Ig_free(entry);$

ERROR: code indent should never use tabs
#206: FILE: util/envlist.c:221:
+^Ipenv = env = g_malloc((envlist->el_count + 1) * sizeof(char *));$

ERROR: code indent should never use tabs
#211: FILE: util/envlist.c:225:
+^I^I*(penv++) = g_strdup(entry->ev_var);$

total: 13 errors, 0 warnings, 168 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
Re: [Qemu-devel] [Qemu-trivial] [PATCH v5] util: Use g_malloc/g_free in envlist.c
Posted by Philippe Mathieu-Daudé 7 years ago
Hi Saurav,

you should read the QEMU Coding Style and replace your tabs by 4 spaces.

On 03/20/2017 02:38 PM, Saurav Sachidanand wrote:
> Change malloc/strdup/free to g_malloc/g_strdup/g_free in
> util/envlist.c.
>
> Remove NULL checks for pointers returned from g_malloc and g_strdup
> as they exit in case of failure. Also, update calls to envlist_create
> to reflect this.
>
> Free array and array contents returned by envlist_to_environ using
> g_free in bsd-user/main.c and linux-user/main.c.
>
> Update comments to reflect change in semantics.
>
> Signed-off-by: Saurav Sachidanand <sauravsachidanand@gmail.com>
> ---
>  bsd-user/main.c   | 14 ++++----------
>  linux-user/main.c |  9 +++------
>  util/envlist.c    | 47 +++++++++++++++++++----------------------------
>  3 files changed, 26 insertions(+), 44 deletions(-)
>
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index 714a692e6f..04f95ddd54 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)
> @@ -956,10 +950,10 @@ int main(int argc, char **argv)
>      }
>
>      for (wrk = target_environ; *wrk; wrk++) {
> -        free(*wrk);
> +        g_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..5f20769cb9 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++) {
> @@ -4429,10 +4426,10 @@ int main(int argc, char **argv, char **envp)
>      }
>
>      for (wrk = target_environ; *wrk; wrk++) {
> -        free(*wrk);
> +        g_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..1eeb7fca87 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;
> @@ -48,10 +46,10 @@ envlist_free(envlist_t *envlist)
>  		entry = envlist->el_entries.lh_first;
>  		QLIST_REMOVE(entry, ev_link);
>
> -		free((char *)entry->ev_var);
> -		free(entry);
> +		g_free((char *)entry->ev_var);
> +		g_free(entry);
>  	}
> -	free(envlist);
> +	g_free(envlist);
>  }
>
>  /*
> @@ -101,8 +99,7 @@ envlist_parse(envlist_t *envlist, const char *env,
>  	if ((envlist == NULL) || (env == NULL))
>  		return (EINVAL);
>
> -	if ((tmpenv = strdup(env)) == NULL)
> -		return (errno);
> +	tmpenv = g_strdup(env);
>      envsave = tmpenv;
>
>      do {
> @@ -117,7 +114,7 @@ envlist_parse(envlist_t *envlist, const char *env,
>          tmpenv = envvar + 1;
>      } while (envvar != NULL);
>
> -    free(envsave);
> +    g_free(envsave);
>      return ret;
>  }
>
> @@ -155,18 +152,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((char *)entry->ev_var);
> +		g_free(entry);
>  	} else {
>  		envlist->el_count++;
>  	}
>
> -	if ((entry = malloc(sizeof (*entry))) == NULL)
> -		return (errno);
> -	if ((entry->ev_var = strdup(env)) == NULL) {
> -		free(entry);
> -		return (errno);
> -	}
> +	entry = g_malloc(sizeof(*entry));
> +	entry->ev_var = g_strdup(env);
>  	QLIST_INSERT_HEAD(&envlist->el_entries, entry, ev_link);
>
>  	return (0);
> @@ -201,8 +194,8 @@ 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((char *)entry->ev_var);
> +		g_free(entry);
>
>  		envlist->el_count--;
>  	}
> @@ -212,12 +205,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 g_free for each element and 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,13 +218,11 @@ 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 *));

use g_malloc_n() which cares "to detect possible overflow during 
multiplication":

penv = env = g_malloc_n(envlist->el_count + 1, sizeof(char *));

>
>  	for (entry = envlist->el_entries.lh_first; entry != NULL;
>  	    entry = entry->ev_link.le_next) {
> -		*(penv++) = strdup(entry->ev_var);
> +		*(penv++) = g_strdup(entry->ev_var);
>  	}
>  	*penv = NULL; /* NULL terminate the list */
>
>

Re: [Qemu-devel] [Qemu-trivial] [PATCH v5] util: Use g_malloc/g_free in envlist.c
Posted by Peter Maydell 7 years ago
On 28 March 2017 at 14:44, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Hi Saurav,
>
> you should read the QEMU Coding Style and replace your tabs by 4 spaces.

Er, in review comments on v3 we said not to do that...

thanks
-- PMM

Re: [Qemu-devel] [Qemu-trivial] [PATCH v5] util: Use g_malloc/g_free in envlist.c
Posted by Philippe Mathieu-Daudé 6 years, 11 months ago
On 03/28/2017 10:47 AM, Peter Maydell wrote:
> On 28 March 2017 at 14:44, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> Hi Saurav,
>>
>> you should read the QEMU Coding Style and replace your tabs by 4 spaces.
>
> Er, in review comments on v3 we said not to do that...

oops I missed Stefan comment in v2, sorry!

>
> thanks
> -- PMM
>

Re: [Qemu-devel] [Qemu-trivial] [PATCH v5] util: Use g_malloc/g_free in envlist.c
Posted by Philippe Mathieu-Daudé 6 years, 11 months ago
On 03/20/2017 02:38 PM, Saurav Sachidanand wrote:
> Change malloc/strdup/free to g_malloc/g_strdup/g_free in
> util/envlist.c.
>
> Remove NULL checks for pointers returned from g_malloc and g_strdup
> as they exit in case of failure. Also, update calls to envlist_create
> to reflect this.
>
> Free array and array contents returned by envlist_to_environ using
> g_free in bsd-user/main.c and linux-user/main.c.
>
> Update comments to reflect change in semantics.
>
> Signed-off-by: Saurav Sachidanand <sauravsachidanand@gmail.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  bsd-user/main.c   | 14 ++++----------
>  linux-user/main.c |  9 +++------
>  util/envlist.c    | 47 +++++++++++++++++++----------------------------
>  3 files changed, 26 insertions(+), 44 deletions(-)
>
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index 714a692e6f..04f95ddd54 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)
> @@ -956,10 +950,10 @@ int main(int argc, char **argv)
>      }
>
>      for (wrk = target_environ; *wrk; wrk++) {
> -        free(*wrk);
> +        g_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..5f20769cb9 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++) {
> @@ -4429,10 +4426,10 @@ int main(int argc, char **argv, char **envp)
>      }
>
>      for (wrk = target_environ; *wrk; wrk++) {
> -        free(*wrk);
> +        g_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..1eeb7fca87 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;
> @@ -48,10 +46,10 @@ envlist_free(envlist_t *envlist)
>  		entry = envlist->el_entries.lh_first;
>  		QLIST_REMOVE(entry, ev_link);
>
> -		free((char *)entry->ev_var);
> -		free(entry);
> +		g_free((char *)entry->ev_var);
> +		g_free(entry);
>  	}
> -	free(envlist);
> +	g_free(envlist);
>  }
>
>  /*
> @@ -101,8 +99,7 @@ envlist_parse(envlist_t *envlist, const char *env,
>  	if ((envlist == NULL) || (env == NULL))
>  		return (EINVAL);
>
> -	if ((tmpenv = strdup(env)) == NULL)
> -		return (errno);
> +	tmpenv = g_strdup(env);
>      envsave = tmpenv;
>
>      do {
> @@ -117,7 +114,7 @@ envlist_parse(envlist_t *envlist, const char *env,
>          tmpenv = envvar + 1;
>      } while (envvar != NULL);
>
> -    free(envsave);
> +    g_free(envsave);
>      return ret;
>  }
>
> @@ -155,18 +152,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((char *)entry->ev_var);
> +		g_free(entry);
>  	} else {
>  		envlist->el_count++;
>  	}
>
> -	if ((entry = malloc(sizeof (*entry))) == NULL)
> -		return (errno);
> -	if ((entry->ev_var = strdup(env)) == NULL) {
> -		free(entry);
> -		return (errno);
> -	}
> +	entry = g_malloc(sizeof(*entry));
> +	entry->ev_var = g_strdup(env);
>  	QLIST_INSERT_HEAD(&envlist->el_entries, entry, ev_link);
>
>  	return (0);
> @@ -201,8 +194,8 @@ 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((char *)entry->ev_var);
> +		g_free(entry);
>
>  		envlist->el_count--;
>  	}
> @@ -212,12 +205,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 g_free for each element and 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,13 +218,11 @@ 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) {
> -		*(penv++) = strdup(entry->ev_var);
> +		*(penv++) = g_strdup(entry->ev_var);
>  	}
>  	*penv = NULL; /* NULL terminate the list */
>
>

Re: [Qemu-devel] [PATCH v5] util: Use g_malloc/g_free in envlist.c
Posted by Michael Tokarev 6 years, 11 months ago
20.03.2017 20:38, Saurav Sachidanand wrote:
> Change malloc/strdup/free to g_malloc/g_strdup/g_free in
> util/envlist.c.
> 
> Remove NULL checks for pointers returned from g_malloc and g_strdup
> as they exit in case of failure. Also, update calls to envlist_create
> to reflect this.
> 
> Free array and array contents returned by envlist_to_environ using
> g_free in bsd-user/main.c and linux-user/main.c.

Applied to -trivial, as-is, with the tabs instead of spaces.

Thanks,

/mjt