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

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

[Prerna Garg]

[-- Attachment #1: Type: text/plain, Size: 2 bytes --]



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0003-Changed-malloc-and-free-to-g_malloc-and-g_free-respe.patch --]
[-- Type: text/x-patch; name="0003-Changed-malloc-and-free-to-g_malloc-and-g_free-respe.patch", Size: 2169 bytes --]

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

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 3176 bytes --]

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.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 735 bytes --]

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

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 567 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]