[B.A.T.M.A.N.] [PATCH] alfred: free globals. Fixes memory leak

[B.A.T.M.A.N.] [PATCH] alfred: free globals. Fixes memory leak

[Nils Schneider]

---
 main.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/main.c b/main.c
index 86acfb7..58622f8 100644
--- a/main.c
+++ b/main.c
@@ -142,6 +142,7 @@ static struct globals *alfred_init(int argc, char *argv[])
 int main(int argc, char *argv[])
 {
 	struct globals *globals;
+	int ret;
 
 	globals = alfred_init(argc, argv);
 
@@ -150,15 +151,21 @@ int main(int argc, char *argv[])
 
 	switch (globals->clientmode) {
 	case CLIENT_NONE:
-		return alfred_server(globals);
+		ret = alfred_server(globals);
 		break;
 	case CLIENT_REQUEST_DATA:
-		return alfred_client_request_data(globals);
+		ret = alfred_client_request_data(globals);
 		break;
 	case CLIENT_SET_DATA:
-		return alfred_client_set_data(globals);
+		ret = alfred_client_set_data(globals);
 		break;
+	default:
+		ret = 0;
 	}
 
-	return 0;
+	free(globals->interface);
+	free(globals->mesh_iface);
+	free(globals);
+
+	return ret;
 }
-- 
1.8.4

Re: [B.A.T.M.A.N.] [PATCH] alfred: free globals. Fixes memory leak

[Simon Wunderlich]

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

On Mon, Sep 09, 2013 at 03:39:02AM +0200, Nils Schneider wrote:
> ---
>  main.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/main.c b/main.c
> index 86acfb7..58622f8 100644
> --- a/main.c
> +++ b/main.c
> @@ -142,6 +142,7 @@ static struct globals *alfred_init(int argc, char *argv[])
>  int main(int argc, char *argv[])
>  {
>  	struct globals *globals;
> +	int ret;
>  
>  	globals = alfred_init(argc, argv);
>  
> @@ -150,15 +151,21 @@ int main(int argc, char *argv[])
>  
>  	switch (globals->clientmode) {
>  	case CLIENT_NONE:
> -		return alfred_server(globals);
> +		ret = alfred_server(globals);
>  		break;
>  	case CLIENT_REQUEST_DATA:
> -		return alfred_client_request_data(globals);
> +		ret = alfred_client_request_data(globals);
>  		break;
>  	case CLIENT_SET_DATA:
> -		return alfred_client_set_data(globals);
> +		ret = alfred_client_set_data(globals);
>  		break;
> +	default:
> +		ret = 0;
>  	}
>  
> -	return 0;
> +	free(globals->interface);
> +	free(globals->mesh_iface);
> +	free(globals);
> +
> +	return ret;

Hrm, apart from that this patch lacks a commit message and signoff, what does it help
freeing stuff just before the program exits? This will free anything automatically
anyway ...

Cheers,
	Simon

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [B.A.T.M.A.N.] [PATCH] alfred: free globals. Fixes memory leak

[Linus Lüssing]

On Mon, Sep 09, 2013 at 05:08:31PM +0200, Simon Wunderlich wrote:
> Hrm, apart from that this patch lacks a commit message and signoff, what does it help
> freeing stuff just before the program exits? This will free anything automatically
> anyway ...
> 

Hm, the advantage of freeing such memory in the code is that
for one thing it makes valgrind happy :P. And this in turn makes
finding "real" memory leaks easier. And besides, isn't it a better
coding style to free such global variables, too?

I think I'd love seeing a PATCHv2 of this one. And I guess only a
short, two sentence commit message should be sufficient as this
isn't a very invasive patch. For instance:

"This patch fixes a non-serious memory leak on program exit by
freeing the memory allocated for the globals variables. This makes
valgrind happy, too.

Signed-off-by: ..."


Cheers, Linus


PS: Sorry, I had a glance over this patch before it got submitted
but even though I should be somehow familiar with the prefered
coding style, I didn't object.