| Message ID | BM1PR01MB0722EAF9BB6F0804187CF81984000@BM1PR01MB0722.INDPRD01.PROD.OUTLOOK.COM |
|---|---|
| State | New |
| Headers | show |
On 04/11/2017 03:41 PM, Prerna Garg wrote: > [Nothing - it was all in the attachment] > 0003-Changed-malloc-and-free-to-g_malloc-and-g_free-respe.patch > While attachments are probably okay for a one-shot contribution, if your intent is to become an active contributor for Outreachy, you'll definitely want to fix your outgoing mail setup to work with inline patches sent directly from 'git send-email' rather than forcing everyone else to deal with your unusual workflow. > > From f3ee3cf9b69aca86c78ec26468a0b4533744cdad Mon Sep 17 00:00:00 2001 > From: Prerna Garg <prerna.garg@live.com> > Date: Wed, 12 Apr 2017 02:04:36 +0530 > Subject: [PATCH 3/3] Changed malloc and free to g_malloc and g_free > respectively in util/envlist.c file Is there a patch 1/3 and 2/3 (and if so, where's the cover letter 0/3)? Or did you mean for this to be a single-patch series, at version 3? Given that the content has gone by before, I suspect the latter; to achieve that, use 'git send-email -v3'. You're also missing a category, and patches should generally favor imperative tense (think "[apply this patch to] do this") rather than past tense ("[this patch] did this"). Also, you want the subject line to be a short how; the commit body can be used for further explanations. So a better commit message might be: util: Switch malloc to g_malloc in envlist.c Also switch matching free to g_free. Done as one of the suggested bite sized tasks > > Signed-off-by: Prerna Garg <prerna.garg@live.com> > --- > 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); g_malloc() can't fail (rather, it fails by exit()ing), so any code checking for NULL is now dead code, and should be cleaned up as part of the same conversion patch. > > 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); Is the cast still necessary? Are we casting away const? It's useful to mention why a cast is present, if it is necessary; otherwise omit it as part of cleaning up the code. > @@ -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 *)); g_new() is better than g_malloc() if you are going to multiply two values, to make sure you don't overflow the multiplication.
On 04/11/2017 04:30 PM, Eric Blake wrote: > You're also missing a category, and patches should generally favor > imperative tense (think "[apply this patch to] do this") rather than > past tense ("[this patch] did this"). Also, you want the subject line > to be a short how; the commit body can be used for further explanations. I hit send too soon; I was trying to say: the subject line is a short "what"; the commit body goes into "why" or more details "how" as needed. At any rate, these suggestions, and more, can be found at: http://wiki.qemu-project.org/Contribute/SubmitAPatch
Hi Prerna, Saurav Sachidanand recently sent a patch to do the same thing: https://patchwork.ozlabs.org/patch/741100/ Unfortunately it looks like this is duplicated work. Saurav's patch will probably be merged since it's already been through review. The BiteSizedTasks page advises: "Before starting on one of these tasks, it would be wise to double-check the list archives to ensure no one else has recently submitted similar cleanups for the same task." You can search the mailing list archives here: https://lists.nongnu.org/archive/html/qemu-devel/ Stefan
From f3ee3cf9b69aca86c78ec26468a0b4533744cdad Mon Sep 17 00:00:00 2001 From: Prerna Garg <prerna.garg@live.com> Date: Wed, 12 Apr 2017 02:04:36 +0530 Subject: [PATCH 3/3] Changed malloc and free to g_malloc and g_free respectively in util/envlist.c file Signed-off-by: Prerna Garg <prerna.garg@live.com> --- 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); -- 2.7.4