Re: [Qemu-devel] [PATCH] Changed malloc and free to g_malloc and g_free in util/envlist.c

[Eric Blake]

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 <address@hidden>
> 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 <address@hidden>
> ---
>  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.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature