* [Qemu-devel] [Qemu-trivial] [PATCH v5] util: Use g_malloc/g_free in envlist.c
@ 2017-03-20 17:38 Saurav Sachidanand
2017-03-23 8:34 ` Stefan Hajnoczi
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Saurav Sachidanand @ 2017-03-20 17:38 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, stefanha, laurent, Saurav Sachidanand
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH v5] util: Use g_malloc/g_free in envlist.c
2017-03-20 17:38 [Qemu-devel] [Qemu-trivial] [PATCH v5] util: Use g_malloc/g_free in envlist.c Saurav Sachidanand
@ 2017-03-23 8:34 ` Stefan Hajnoczi
2017-03-28 13:26 ` no-reply
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Stefan Hajnoczi @ 2017-03-23 8:34 UTC (permalink / raw)
To: Saurav Sachidanand; +Cc: qemu-devel, qemu-trivial, laurent
[-- Attachment #1: Type: text/plain, Size: 838 bytes --]
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>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH v5] util: Use g_malloc/g_free in envlist.c
2017-03-20 17:38 [Qemu-devel] [Qemu-trivial] [PATCH v5] util: Use g_malloc/g_free in envlist.c Saurav Sachidanand
2017-03-23 8:34 ` Stefan Hajnoczi
@ 2017-03-28 13:26 ` no-reply
2017-03-28 13:44 ` Philippe Mathieu-Daudé
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: no-reply @ 2017-03-28 13:26 UTC (permalink / raw)
To: sauravsachidanand; +Cc: famz, qemu-devel, qemu-trivial, stefanha, laurent
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH v5] util: Use g_malloc/g_free in envlist.c
2017-03-20 17:38 [Qemu-devel] [Qemu-trivial] [PATCH v5] util: Use g_malloc/g_free in envlist.c Saurav Sachidanand
2017-03-23 8:34 ` Stefan Hajnoczi
2017-03-28 13:26 ` no-reply
@ 2017-03-28 13:44 ` Philippe Mathieu-Daudé
2017-03-28 13:47 ` Peter Maydell
2017-04-04 10:19 ` Philippe Mathieu-Daudé
2017-04-23 9:01 ` [Qemu-devel] " Michael Tokarev
4 siblings, 1 reply; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-03-28 13:44 UTC (permalink / raw)
To: Saurav Sachidanand; +Cc: qemu-devel, qemu-trivial, stefanha, laurent
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 */
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH v5] util: Use g_malloc/g_free in envlist.c
2017-03-28 13:44 ` Philippe Mathieu-Daudé
@ 2017-03-28 13:47 ` Peter Maydell
2017-04-04 10:15 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 8+ messages in thread
From: Peter Maydell @ 2017-03-28 13:47 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Saurav Sachidanand, QEMU Trivial, Stefan Hajnoczi,
QEMU Developers, Laurent Vivier
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH v5] util: Use g_malloc/g_free in envlist.c
2017-03-28 13:47 ` Peter Maydell
@ 2017-04-04 10:15 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-04-04 10:15 UTC (permalink / raw)
To: Peter Maydell
Cc: Saurav Sachidanand, QEMU Trivial, Stefan Hajnoczi,
QEMU Developers, Laurent Vivier
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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH v5] util: Use g_malloc/g_free in envlist.c
2017-03-20 17:38 [Qemu-devel] [Qemu-trivial] [PATCH v5] util: Use g_malloc/g_free in envlist.c Saurav Sachidanand
` (2 preceding siblings ...)
2017-03-28 13:44 ` Philippe Mathieu-Daudé
@ 2017-04-04 10:19 ` Philippe Mathieu-Daudé
2017-04-23 9:01 ` [Qemu-devel] " Michael Tokarev
4 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-04-04 10:19 UTC (permalink / raw)
To: Saurav Sachidanand, qemu-devel; +Cc: qemu-trivial, stefanha, laurent
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 */
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v5] util: Use g_malloc/g_free in envlist.c
2017-03-20 17:38 [Qemu-devel] [Qemu-trivial] [PATCH v5] util: Use g_malloc/g_free in envlist.c Saurav Sachidanand
` (3 preceding siblings ...)
2017-04-04 10:19 ` Philippe Mathieu-Daudé
@ 2017-04-23 9:01 ` Michael Tokarev
4 siblings, 0 replies; 8+ messages in thread
From: Michael Tokarev @ 2017-04-23 9:01 UTC (permalink / raw)
To: Saurav Sachidanand, qemu-devel; +Cc: qemu-trivial, stefanha, laurent
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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-04-23 9:01 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-20 17:38 [Qemu-devel] [Qemu-trivial] [PATCH v5] util: Use g_malloc/g_free in envlist.c Saurav Sachidanand
2017-03-23 8:34 ` Stefan Hajnoczi
2017-03-28 13:26 ` no-reply
2017-03-28 13:44 ` Philippe Mathieu-Daudé
2017-03-28 13:47 ` Peter Maydell
2017-04-04 10:15 ` Philippe Mathieu-Daudé
2017-04-04 10:19 ` Philippe Mathieu-Daudé
2017-04-23 9:01 ` [Qemu-devel] " Michael Tokarev
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.