* [Qemu-devel] [PATCH][Outreachy]
@ 2016-03-05 6:45 Sarah Khan
2016-03-05 18:04 ` Paolo Bonzini
2016-03-06 8:18 ` Alex Bennée
0 siblings, 2 replies; 3+ messages in thread
From: Sarah Khan @ 2016-03-05 6:45 UTC (permalink / raw)
To: qemu-devel, famz; +Cc: Sarah Khan
util/envlist.c:This patch replaces malloc with g_malloc
This replacement was suggested as part of the bite-sized tasks.
Signed-off-by: Sarah Khan <sarahjmi07@gmail.com>
----------
diff --git a/util/envlist.c b/util/envlist.c
index e86857e..0324fe2 100644
--- a/util/envlist.c
+++ b/util/envlist.c
@@ -25,7 +25,7 @@ envlist_create(void)
{
envlist_t *envlist;
- if ((envlist = malloc(sizeof (*envlist))) == NULL)
+ if ((envlist = g_malloc(sizeof (*envlist))) == NULL)
return (NULL);
QLIST_INIT(&envlist->el_entries);
@@ -48,10 +48,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);
}
/*
@@ -155,16 +155,16 @@ 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)
+ if ((entry = g_malloc(sizeof (*entry))) == NULL)
return (errno);
if ((entry->ev_var = strdup(env)) == NULL) {
- free(entry);
+ g_free(entry);
return (errno);
}
QLIST_INSERT_HEAD(&envlist->el_entries, entry, ev_link);
@@ -201,8 +201,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--;
}
@@ -225,7 +225,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 *));
+ penv = env = g_malloc((envlist->el_count + 1) * sizeof (char *));
if (env == NULL)
return (NULL);
---
util/envlist.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/util/envlist.c b/util/envlist.c
index e86857e..0324fe2 100644
--- a/util/envlist.c
+++ b/util/envlist.c
@@ -25,7 +25,7 @@ envlist_create(void)
{
envlist_t *envlist;
- if ((envlist = malloc(sizeof (*envlist))) == NULL)
+ if ((envlist = g_malloc(sizeof (*envlist))) == NULL)
return (NULL);
QLIST_INIT(&envlist->el_entries);
@@ -48,10 +48,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);
}
/*
@@ -155,16 +155,16 @@ 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)
+ if ((entry = g_malloc(sizeof (*entry))) == NULL)
return (errno);
if ((entry->ev_var = strdup(env)) == NULL) {
- free(entry);
+ g_free(entry);
return (errno);
}
QLIST_INSERT_HEAD(&envlist->el_entries, entry, ev_link);
@@ -201,8 +201,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--;
}
@@ -225,7 +225,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 *));
+ penv = env = g_malloc((envlist->el_count + 1) * sizeof (char *));
if (env == NULL)
return (NULL);
--
1.9.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH][Outreachy]
2016-03-05 6:45 [Qemu-devel] [PATCH][Outreachy] Sarah Khan
@ 2016-03-05 18:04 ` Paolo Bonzini
2016-03-06 8:18 ` Alex Bennée
1 sibling, 0 replies; 3+ messages in thread
From: Paolo Bonzini @ 2016-03-05 18:04 UTC (permalink / raw)
To: Sarah Khan, qemu-devel, famz
On 05/03/2016 07:45, Sarah Khan wrote:
> util/envlist.c:This patch replaces malloc with g_malloc
This should be placed in the subject. Please follow the instructions at
http://wiki.qemu.org/Contribute/SubmitAPatch#Submitting_your_Patches to
format the patch correctly.
> - if ((envlist = malloc(sizeof (*envlist))) == NULL)
> + if ((envlist = g_malloc(sizeof (*envlist))) == NULL)
> return (NULL);
g_malloc will never return NULL, so you can remove the if.
> - if ((entry = malloc(sizeof (*entry))) == NULL)
> + if ((entry = g_malloc(sizeof (*entry))) == NULL)
> return (errno);
Likewise.
> if ((entry->ev_var = strdup(env)) == NULL) {
You could also replace strdup with g_strdup...
> - free(entry);
> + g_free(entry);
> return (errno);
> }
> QLIST_INSERT_HEAD(&envlist->el_entries, entry, ev_link);
> @@ -201,8 +201,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);
... because here you are replacing its freeing with g_free.
> + g_free(entry);
>
> envlist->el_count--;
> }
> @@ -225,7 +225,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 *));
> + penv = env = g_malloc((envlist->el_count + 1) * sizeof (char *));
> if (env == NULL)
> return (NULL);
This if can be removed.
Note that you have included the patch twice in your message.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH][Outreachy]
2016-03-05 6:45 [Qemu-devel] [PATCH][Outreachy] Sarah Khan
2016-03-05 18:04 ` Paolo Bonzini
@ 2016-03-06 8:18 ` Alex Bennée
1 sibling, 0 replies; 3+ messages in thread
From: Alex Bennée @ 2016-03-06 8:18 UTC (permalink / raw)
To: Sarah Khan; +Cc: famz, qemu-devel
Sarah Khan <sarahjmi07@gmail.com> writes:
> util/envlist.c:This patch replaces malloc with g_malloc
>
> This replacement was suggested as part of the bite-sized tasks.
>
> Signed-off-by: Sarah Khan <sarahjmi07@gmail.com>
Thanks for your contribution. A few notes for the next revision.
It's always worth running $SRC/scripts/checkpatch.pl to check for style
issues. While there are bits of the code base that don't conform we tend
to fix up style issues with the code that is being changed as we go.
>
> ----------
> diff --git a/util/envlist.c b/util/envlist.c
> index e86857e..0324fe2 100644
> --- a/util/envlist.c
> +++ b/util/envlist.c
> @@ -25,7 +25,7 @@ envlist_create(void)
> {
> envlist_t *envlist;
>
> - if ((envlist = malloc(sizeof (*envlist))) == NULL)
> + if ((envlist = g_malloc(sizeof (*envlist))) == NULL)
> return (NULL);
g_malloc has different behaviour from malloc in so far as it never fails
(or rather if it does you program aborts). This means a lot of
boilerplate checking can be simplified.
>
> QLIST_INIT(&envlist->el_entries);
> @@ -48,10 +48,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);
> }
>
> /*
> @@ -155,16 +155,16 @@ 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)
> + if ((entry = g_malloc(sizeof (*entry))) == NULL)
> return (errno);
> if ((entry->ev_var = strdup(env)) == NULL) {
If you're cleaning up the memory allocation you should probably also use
g_strdup() here for the string allocation (it gets g_free'd later).
> - free(entry);
> + g_free(entry);
> return (errno);
> }
> QLIST_INSERT_HEAD(&envlist->el_entries, entry, ev_link);
> @@ -201,8 +201,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--;
> }
> @@ -225,7 +225,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 *));
> + penv = env = g_malloc((envlist->el_count + 1) * sizeof (char *));
> if (env == NULL)
> return (NULL);
Again this checking is now redundant.
> ---
> util/envlist.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/util/envlist.c b/util/envlist.c
> index e86857e..0324fe2 100644
> --- a/util/envlist.c
> +++ b/util/envlist.c
> @@ -25,7 +25,7 @@ envlist_create(void)
> {
> envlist_t *envlist;
>
> - if ((envlist = malloc(sizeof (*envlist))) == NULL)
> + if ((envlist = g_malloc(sizeof (*envlist))) == NULL)
> return (NULL);
>
> QLIST_INIT(&envlist->el_entries);
> @@ -48,10 +48,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);
> }
>
> /*
> @@ -155,16 +155,16 @@ 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)
> + if ((entry = g_malloc(sizeof (*entry))) == NULL)
> return (errno);
> if ((entry->ev_var = strdup(env)) == NULL) {
> - free(entry);
> + g_free(entry);
> return (errno);
> }
> QLIST_INSERT_HEAD(&envlist->el_entries, entry, ev_link);
> @@ -201,8 +201,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--;
> }
> @@ -225,7 +225,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 *));
> + penv = env = g_malloc((envlist->el_count + 1) * sizeof (char *));
> if (env == NULL)
> return (NULL);
Same here.
Thanks for you submission. Feel free to CC me on your next version.
--
Alex Bennée
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-03-06 8:18 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-05 6:45 [Qemu-devel] [PATCH][Outreachy] Sarah Khan
2016-03-05 18:04 ` Paolo Bonzini
2016-03-06 8:18 ` Alex Bennée
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).