Re: [PATCH] fix memory leaks

[Johan Hedberg <johan.hedberg@gmail.com>]

Hi Steve,

Thanks for the excellent reviews!

There are a few changes however that I'd still do to the patch:

On Fri, Sep 25, 2009, Steve Grubb wrote:
>  	if (!record) {
>  		error("Unable to allocate new service record");
> +		g_free(server);
> +		sdp_record_free(record);
>  		return -1;
>  	}

In this branch record is NULL so I guess the sdp_record_free call is a
mistake (it also doesn't handle NULL nicely like some GLib free functions
do and would cause an imediate segfault).

> --- bluez-4.54.orig/src/glib-helper.c	2009-09-25 11:33:47.000000000 -0400
> +++ bluez-4.54/src/glib-helper.c	2009-09-25 16:07:22.000000000 -0400
> @@ -704,7 +704,7 @@ int bt_acl_encrypt(const bdaddr_t *src, 
>  			bt_hci_result_t cb, gpointer user_data)
>  {
>  	GIOChannel *io;
> -	struct hci_cmd_data *cmd;
> +	struct hci_cmd_data *cmd = NULL;
>  	struct hci_conn_info_req *cr;
>  	auth_requested_cp cp;
>  	struct hci_filter nf;
> @@ -778,6 +778,7 @@ int bt_acl_encrypt(const bdaddr_t *src, 
>  	return 0;
>  
>  failed:
> +	g_free(cmd);
>  	close(dd);

As David already mentioned in the other email, it's good to to avoid
initialization upon declaration to help the compiler detect unused
variables. I think it'd be better to add another label right above failed,
e.g. failed_cmd, which does the g_free and then jump to it from those
places in the function that need it.

Johan