[PATCH BlueZ 00/12] Redundant D-Bus error init, remove unwanted code, g_free missmatch etc

[PATCH BlueZ 00/12] Redundant D-Bus error init, remove unwanted code, g_free missmatch etc

[Syam Sidhardhan]

This series of patches resolves:
Memory leak, free - g_free mismatch, redundant D-Bus error initilization
and remove unwanted code.

Syam Sidhardhan (12):
  monitor: Fix memory leak
  storage: Use g_free() instead of free()
  media: Remove redundant D-Bus error initilization
  telephony-maemo5: Remove redundant D-Bus error initilizations
  telephony-maemo6: Remove redundant D-Bus error initilization
  telephony-ofono: Remove redundant D-Bus error initilization
  agent: Remove redundant D-Bus error initilizations
  Remove redundant D-Bus error initilization in maemo6 plugin
  avctp: Fix NULL check after dereference
  adapter: Remove unwanted if check and code
  audio: Remove unwanted code from manager
  input: Remove unwanted codes from device

 audio/avctp.c            |    3 ++-
 audio/manager.c          |    2 --
 audio/media.c            |    1 -
 audio/telephony-maemo5.c |    5 -----
 audio/telephony-maemo6.c |    1 -
 audio/telephony-ofono.c  |    1 -
 input/device.c           |    5 -----
 monitor/hcidump.c        |    3 ++-
 plugins/maemo6.c         |    1 -
 src/adapter.c            |    3 ---
 src/agent.c              |    5 -----
 src/storage.c            |    6 +++---
 12 files changed, 7 insertions(+), 29 deletions(-)

-- 
1.7.4.1

[PATCH BlueZ 01/12] monitor: Fix memory leak

[Syam Sidhardhan]

---
 monitor/hcidump.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/monitor/hcidump.c b/monitor/hcidump.c
index 043c75e..373d2f5 100644
--- a/monitor/hcidump.c
+++ b/monitor/hcidump.c
@@ -233,7 +233,7 @@ static void device_list(int fd, int max_dev)
 
 	if (ioctl(fd, HCIGETDEVLIST, (void *) dl) < 0) {
 		perror("Failed to get device list");
-		return;
+		goto done;
 	}
 
 	for (i = 0; i < dl->dev_num; i++, dr++) {
@@ -253,6 +253,7 @@ static void device_list(int fd, int max_dev)
 		open_device(dr->dev_id);
 	}
 
+done:
 	free(dl);
 }
 
-- 
1.7.4.1

[PATCH BlueZ 02/12] storage: Use g_free() instead of free()

[Syam Sidhardhan]

It is important to match g_*() memory allocation with g_free()
---
As per the glib documentation, the memory allocated using the glib
api's needs to be freed using corresponding glib free api's and not
libc api's, otherwise undefined behaviour can happen. This is because
the libc allocation and glib allocation uses different memory pools.

 src/storage.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/storage.c b/src/storage.c
index 973d545..a91ee2d 100644
--- a/src/storage.c
+++ b/src/storage.c
@@ -732,7 +732,7 @@ int write_trust(const char *src, const char *addr, const char *service,
 	else {
 		char *new_str = service_list_to_string(services);
 		ret = textfile_caseput(filename, addr, new_str);
-		free(new_str);
+		g_free(new_str);
 	}
 
 	g_slist_free(services);
@@ -817,7 +817,7 @@ int store_record(const gchar *src, const gchar *dst, sdp_record_t *rec)
 	err = textfile_put(filename, key, str);
 
 	free(buf.data);
-	free(str);
+	g_free(str);
 
 	return err;
 }
@@ -839,7 +839,7 @@ sdp_record_t *record_from_string(const gchar *str)
 	}
 
 	rec = sdp_extract_pdu(pdata, size, &len);
-	free(pdata);
+	g_free(pdata);
 
 	return rec;
 }
-- 
1.7.4.1

[PATCH BlueZ 03/12] media: Remove redundant D-Bus error initilization

[Syam Sidhardhan]

---
 audio/media.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/audio/media.c b/audio/media.c
index fdd7b49..cb8872f 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -276,7 +276,6 @@ static void endpoint_reply(DBusPendingCall *call, void *user_data)
 		goto done;
 	}
 
-	dbus_error_init(&err);
 	if (dbus_message_is_method_call(request->msg, MEDIA_ENDPOINT_INTERFACE,
 				"SelectConfiguration")) {
 		DBusMessageIter args, array;
-- 
1.7.4.1

[PATCH BlueZ 04/12] telephony-maemo5: Remove redundant D-Bus error initilizations

[Syam Sidhardhan]

---
 audio/telephony-maemo5.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/audio/telephony-maemo5.c b/audio/telephony-maemo5.c
index 8a00296..18e4773 100644
--- a/audio/telephony-maemo5.c
+++ b/audio/telephony-maemo5.c
@@ -1195,7 +1195,6 @@ static void get_operator_name_reply(DBusPendingCall *pending_call,
 		goto done;
 	}
 
-	dbus_error_init(&err);
 	if (!dbus_message_get_args(reply, &err,
 					DBUS_TYPE_STRING, &name,
 					DBUS_TYPE_INT32, &net_err,
@@ -1402,7 +1401,6 @@ static void hal_battery_level_reply(DBusPendingCall *call, void *user_data)
 		goto done;
 	}
 
-	dbus_error_init(&err);
 	if (dbus_message_get_args(reply, &err,
 				DBUS_TYPE_INT32, &level,
 				DBUS_TYPE_INVALID) == FALSE) {
@@ -1590,7 +1588,6 @@ static void signal_strength_reply(DBusPendingCall *call, void *user_data)
 		goto done;
 	}
 
-	dbus_error_init(&err);
 	if (!dbus_message_get_args(reply, &err,
 					DBUS_TYPE_BYTE, &signals_bar,
 					DBUS_TYPE_BYTE, &rssi_in_dbm,
@@ -1640,7 +1637,6 @@ static void registration_status_reply(DBusPendingCall *call, void *user_data)
 		goto done;
 	}
 
-	dbus_error_init(&err);
 	if (!dbus_message_get_args(reply, &err,
 					DBUS_TYPE_BYTE, &status,
 					DBUS_TYPE_UINT16, &lac,
@@ -1787,7 +1783,6 @@ static void phonebook_read_reply(DBusPendingCall *call, void *user_data)
 		goto done;
 	}
 
-	dbus_error_init(&derr);
 	if (dbus_message_get_args(reply, &derr,
 				DBUS_TYPE_STRING, &name,
 				DBUS_TYPE_STRING, &number,
-- 
1.7.4.1

[PATCH BlueZ 05/12] telephony-maemo6: Remove redundant D-Bus error initilization

[Syam Sidhardhan]

---
 audio/telephony-maemo6.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/audio/telephony-maemo6.c b/audio/telephony-maemo6.c
index 0727ffe..fcfd4a4 100644
--- a/audio/telephony-maemo6.c
+++ b/audio/telephony-maemo6.c
@@ -1911,7 +1911,6 @@ static void phonebook_read_reply(DBusPendingCall *call, void *user_data)
 		goto done;
 	}
 
-	dbus_error_init(&derr);
 	if (dbus_message_get_args(reply, NULL,
 				DBUS_TYPE_INT32, &index,
 				DBUS_TYPE_STRING, &name,
-- 
1.7.4.1

[PATCH BlueZ 06/12] telephony-ofono: Remove redundant D-Bus error initilization

[Syam Sidhardhan]

---
 audio/telephony-ofono.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/audio/telephony-ofono.c b/audio/telephony-ofono.c
index 961fedd..5e4ca89 100644
--- a/audio/telephony-ofono.c
+++ b/audio/telephony-ofono.c
@@ -1360,7 +1360,6 @@ static void hal_battery_level_reply(DBusPendingCall *call, void *user_data)
 		goto done;
 	}
 
-	dbus_error_init(&err);
 	if (dbus_message_get_args(reply, &err,
 				DBUS_TYPE_INT32, &level,
 				DBUS_TYPE_INVALID) == FALSE) {
-- 
1.7.4.1

[PATCH BlueZ 07/12] agent: Remove redundant D-Bus error initilizations

[Syam Sidhardhan]

---
 src/agent.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/src/agent.c b/src/agent.c
index c628024..579b03e 100644
--- a/src/agent.c
+++ b/src/agent.c
@@ -272,7 +272,6 @@ static void simple_agent_reply(DBusPendingCall *call, void *user_data)
 		goto done;
 	}
 
-	dbus_error_init(&err);
 	if (!dbus_message_get_args(message, &err, DBUS_TYPE_INVALID)) {
 		error("Wrong reply signature: %s", err.message);
 		cb(agent, &err, req->user_data);
@@ -373,7 +372,6 @@ static void pincode_reply(DBusPendingCall *call, void *user_data)
 		goto done;
 	}
 
-	dbus_error_init(&err);
 	if (!dbus_message_get_args(message, &err,
 				DBUS_TYPE_STRING, &pin,
 				DBUS_TYPE_INVALID)) {
@@ -385,7 +383,6 @@ static void pincode_reply(DBusPendingCall *call, void *user_data)
 
 	len = strlen(pin);
 
-	dbus_error_init(&err);
 	if (len > 16 || len < 1) {
 		error("Invalid PIN length (%zu) from agent", len);
 		dbus_set_error_const(&err, "org.bluez.Error.InvalidArgs",
@@ -538,7 +535,6 @@ static void passkey_reply(DBusPendingCall *call, void *user_data)
 		goto done;
 	}
 
-	dbus_error_init(&err);
 	if (!dbus_message_get_args(message, &err,
 				DBUS_TYPE_UINT32, &passkey,
 				DBUS_TYPE_INVALID)) {
@@ -733,7 +729,6 @@ static void display_pincode_reply(DBusPendingCall *call, void *user_data)
 		goto done;
 	}
 
-	dbus_error_init(&err);
 	if (!dbus_message_get_args(message, &err, DBUS_TYPE_INVALID)) {
 		error("Wrong reply signature: %s", err.message);
 		cb(agent, &err, req->user_data);
-- 
1.7.4.1

[PATCH BlueZ 08/12] Remove redundant D-Bus error initilization in maemo6 plugin

[Syam Sidhardhan]

---
 plugins/maemo6.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/plugins/maemo6.c b/plugins/maemo6.c
index 6c34116..4819af4 100644
--- a/plugins/maemo6.c
+++ b/plugins/maemo6.c
@@ -130,7 +130,6 @@ static void read_radio_states_cb(DBusPendingCall *call, void *user_data)
 		goto done;
 	}
 
-	dbus_error_init(&derr);
 	if (dbus_message_get_args(reply, &derr,
 				DBUS_TYPE_UINT32, &radio_states,
 				DBUS_TYPE_INVALID) == FALSE) {
-- 
1.7.4.1

[PATCH BlueZ 09/12] avctp: Fix NULL check after dereference

[Syam Sidhardhan]

Check for session != NULL has to be done before accessing session.
---
 audio/avctp.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/audio/avctp.c b/audio/avctp.c
index d98d097..1bc2a1d 100644
--- a/audio/avctp.c
+++ b/audio/avctp.c
@@ -320,7 +320,7 @@ static struct avctp_pdu_handler *find_handler(GSList *list, uint8_t opcode)
 
 static void avctp_disconnected(struct avctp *session)
 {
-	struct avctp_server *server = session->server;
+	struct avctp_server *server;
 
 	if (!session)
 		return;
@@ -356,6 +356,7 @@ static void avctp_disconnected(struct avctp *session)
 		session->uinput = -1;
 	}
 
+	server = session->server;
 	server->sessions = g_slist_remove(server->sessions, session);
 	g_slist_free_full(session->handlers, g_free);
 	g_free(session);
-- 
1.7.4.1

[PATCH BlueZ 10/12] adapter: Remove unwanted if check and code

[Syam Sidhardhan]

Here the variable agent never be NULL, so no need to check it
against NULL.
---
 src/adapter.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 18dd5b6..16d379c 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1618,9 +1618,6 @@ static DBusMessage *register_agent(DBusConnection *conn, DBusMessage *msg,
 
 	agent = agent_create(adapter, name, path, cap,
 				(agent_remove_cb) agent_removed, adapter);
-	if (!agent)
-		return btd_error_failed(msg, "Failed to create a new agent");
-
 	adapter->agent = agent;
 
 	DBG("Agent registered for hci%d at %s:%s", adapter->dev_id, name,
-- 
1.7.4.1

[PATCH BlueZ 11/12] audio: Remove unwanted code from manager

[Syam Sidhardhan]

Here the variable adp never be NULL, so no need to check it
against NULL.
---
 audio/manager.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/audio/manager.c b/audio/manager.c
index 9c7d0d3..fb7af98 100644
--- a/audio/manager.c
+++ b/audio/manager.c
@@ -852,8 +852,6 @@ static struct audio_adapter *audio_adapter_get(struct btd_adapter *adapter)
 	adp = find_adapter(adapters, adapter);
 	if (!adp) {
 		adp = audio_adapter_create(adapter);
-		if (!adp)
-			return NULL;
 		adapters = g_slist_append(adapters, adp);
 	} else
 		audio_adapter_ref(adp);
-- 
1.7.4.1

[PATCH BlueZ 12/12] input: Remove unwanted codes from device

[Syam Sidhardhan]

Here the variable iconn never be NULL, so no need to check it
against NULL.
---
 input/device.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/input/device.c b/input/device.c
index 77e77b3..e2c48b9 100644
--- a/input/device.c
+++ b/input/device.c
@@ -1153,8 +1153,6 @@ int input_device_register(DBusConnection *conn, struct btd_device *device,
 	}
 
 	iconn = input_conn_new(idev, uuid, timeout);
-	if (!iconn)
-		return -EINVAL;
 
 	idev->connections = g_slist_append(idev->connections, iconn);
 
@@ -1176,9 +1174,6 @@ int fake_input_register(DBusConnection *conn, struct btd_device *device,
 	}
 
 	iconn = input_conn_new(idev, uuid, 0);
-	if (!iconn)
-		return -EINVAL;
-
 	iconn->fake = g_new0(struct fake_input, 1);
 	iconn->fake->ch = channel;
 	iconn->fake->connect = rfcomm_connect;
-- 
1.7.4.1

Re: [PATCH BlueZ 10/12] adapter: Remove unwanted if check and code

[Vinicius Costa Gomes]

Hi Syam,

On 21:27 Wed 23 May, Syam Sidhardhan wrote:
> Here the variable agent never be NULL, so no need to check it
> against NULL.
> ---
>  src/adapter.c |    3 ---
>  1 files changed, 0 insertions(+), 3 deletions(-)
> 
> diff --git a/src/adapter.c b/src/adapter.c
> index 18dd5b6..16d379c 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -1618,9 +1618,6 @@ static DBusMessage *register_agent(DBusConnection *conn, DBusMessage *msg,
>  
>  	agent = agent_create(adapter, name, path, cap,
>  				(agent_remove_cb) agent_removed, adapter);
> -	if (!agent)
> -		return btd_error_failed(msg, "Failed to create a new agent");
> -

While you are at it, you could remove the "agent" variable.

Apart from this, the series looks good.

>  	adapter->agent = agent;
>  
>  	DBG("Agent registered for hci%d at %s:%s", adapter->dev_id, name,
> -- 
> 1.7.4.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Cheers,
-- 
Vinicius

Re: [PATCH BlueZ 00/12] Redundant D-Bus error init, remove unwanted code, g_free missmatch etc

[Johan Hedberg]

Hi Syam,

On Wed, May 23, 2012, Syam Sidhardhan wrote:
> This series of patches resolves:
> Memory leak, free - g_free mismatch, redundant D-Bus error initilization
> and remove unwanted code.
> 
> Syam Sidhardhan (12):
>   monitor: Fix memory leak
>   storage: Use g_free() instead of free()
>   media: Remove redundant D-Bus error initilization
>   telephony-maemo5: Remove redundant D-Bus error initilizations
>   telephony-maemo6: Remove redundant D-Bus error initilization
>   telephony-ofono: Remove redundant D-Bus error initilization
>   agent: Remove redundant D-Bus error initilizations
>   Remove redundant D-Bus error initilization in maemo6 plugin
>   avctp: Fix NULL check after dereference
>   adapter: Remove unwanted if check and code
>   audio: Remove unwanted code from manager
>   input: Remove unwanted codes from device
> 
>  audio/avctp.c            |    3 ++-
>  audio/manager.c          |    2 --
>  audio/media.c            |    1 -
>  audio/telephony-maemo5.c |    5 -----
>  audio/telephony-maemo6.c |    1 -
>  audio/telephony-ofono.c  |    1 -
>  input/device.c           |    5 -----
>  monitor/hcidump.c        |    3 ++-
>  plugins/maemo6.c         |    1 -
>  src/adapter.c            |    3 ---
>  src/agent.c              |    5 -----
>  src/storage.c            |    6 +++---
>  12 files changed, 7 insertions(+), 29 deletions(-)

I've applied all patches except the telephony ones. Those do not buy us
very much but come with the expense of conflicting with the pending
telephony driver removal patches (destined for 5.0). I also applied one
extra patch from myself to do the agent variable removal suggested by
Vinicius.

Johan