linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [Bluez-devel] [PATCH] dbus.c cleanup
@ 2005-10-29 17:53 Johan Hedberg
  2005-10-29 18:07 ` Marcel Holtmann
  0 siblings, 1 reply; 27+ messages in thread
From: Johan Hedberg @ 2005-10-29 17:53 UTC (permalink / raw)
  To: bluez-devel

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

Hi,

Here's a cleanup patch for the hcid dbus code. It changes several
sprintf calls to snprintf (better to risk string truncation than buffer
overflow), as well as removes unecessary byte parameters from the replys
of periodic inquiry methods.

Johan

[-- Attachment #2: dbus-cleanup.patch --]
[-- Type: text/plain, Size: 5104 bytes --]

Index: dbus.c
===================================================================
RCS file: /cvsroot/bluez/utils/hcid/dbus.c,v
retrieving revision 1.36
diff -u -r1.36 dbus.c
--- dbus.c	27 Oct 2005 17:08:13 -0000	1.36
+++ dbus.c	29 Oct 2005 17:44:37 -0000
@@ -886,12 +886,12 @@
 
 		for (; *fst_level; fst_level++) {
 			ptr1 = *fst_level;
-			sprintf(snd_parent, "%s/%s", fst_parent, ptr1);
+			snprintf(snd_parent, sizeof(snd_parent), "%s/%s", fst_parent, ptr1);
 
 			if (dbus_connection_list_registered(connection, snd_parent, &snd_level)) {
 
 				if (!(*snd_level)) {
-					sprintf(path, "%s/%s", MANAGER_PATH, ptr1);
+					snprintf(path, sizeof(path), "%s/%s", MANAGER_PATH, ptr1);
 
 					if (dbus_connection_get_object_path_data(connection,
 								path, &data)) {
@@ -909,7 +909,7 @@
 
 				for (; *snd_level; snd_level++) {
 					ptr2 = *snd_level;
-					sprintf(path, "%s/%s/%s", MANAGER_PATH, ptr1, ptr2);
+					snprintf(path, sizeof(path), "%s/%s/%s", MANAGER_PATH, ptr1, ptr2);
 
 					if (dbus_connection_get_object_path_data(connection,
 								path, &data)) {
@@ -935,12 +935,12 @@
 
 gboolean hcid_dbus_register_device(uint16_t id) 
 {
-	char path[MAX_PATH_LENGTH+1];
-	char dev[BLUETOOTH_DEVICE_NAME_LEN+1];
+	char path[MAX_PATH_LENGTH];
+	char dev[BLUETOOTH_DEVICE_NAME_LEN];
 	const char *pdev = dev;
 
-	snprintf(dev, BLUETOOTH_DEVICE_NAME_LEN, HCI_DEVICE_NAME "%d", id);
-	snprintf(path, MAX_PATH_LENGTH, "%s/%s", DEVICE_PATH, pdev);
+	snprintf(dev, sizeof(dev), HCI_DEVICE_NAME "%d", id);
+	snprintf(path, sizeof(path), "%s/%s", DEVICE_PATH, pdev);
 
 	/* register the default path*/
 	return register_dbus_path(path, id);
@@ -948,12 +948,12 @@
 
 gboolean hcid_dbus_unregister_device(uint16_t id)
 {
-	char dev[BLUETOOTH_DEVICE_NAME_LEN+1];
-	char path[MAX_PATH_LENGTH+1];
+	char dev[BLUETOOTH_DEVICE_NAME_LEN];
+	char path[MAX_PATH_LENGTH];
 	const char *pdev = dev;
 
-	snprintf(dev, BLUETOOTH_DEVICE_NAME_LEN, HCI_DEVICE_NAME "%d", id);
-	snprintf(path, MAX_PATH_LENGTH, "%s/%s", DEVICE_PATH, pdev);
+	snprintf(dev, sizeof(dev), HCI_DEVICE_NAME "%d", id);
+	snprintf(path, sizeof(path), "%s/%s", DEVICE_PATH, pdev);
 
 	return unregister_dbus_path(path);
 }
@@ -986,7 +986,7 @@
 		goto failed;
 	}
 
-	sprintf(dev, HCI_DEVICE_NAME "%d", id);
+	snprintf(dev, sizeof(dev), HCI_DEVICE_NAME "%d", id);
 
 	dbus_message_iter_init_append(message, &iter);
 	dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING ,&pdev);
@@ -1036,7 +1036,7 @@
 		goto failed;
 	}
 
-	sprintf(dev, HCI_DEVICE_NAME "%d", id);
+	snprintf(dev, sizeof(dev), HCI_DEVICE_NAME "%d", id);
 
 	dbus_message_iter_init_append(message, &iter);
 	dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING ,&pdev);
@@ -1072,12 +1072,12 @@
 
 	/* register the default path*/
 	if (!dft_reg) {
-		sprintf(path, "%s/%s/%s", MANAGER_PATH, HCI_DEFAULT_DEVICE_NAME, BLUEZ_HCI);
+		snprintf(path, sizeof(path), "%s/%s/%s", MANAGER_PATH, HCI_DEFAULT_DEVICE_NAME, BLUEZ_HCI);
 		register_dbus_path(path, DEFAULT_DEVICE_PATH_ID);
 	}
 
 	/* register the default path*/
-	sprintf(path, "%s/%s%d/%s", MANAGER_PATH, HCI_DEVICE_NAME, id, BLUEZ_HCI);
+	snprintf(path, sizeof(path), "%s/%s%d/%s", MANAGER_PATH, HCI_DEVICE_NAME, id, BLUEZ_HCI);
 	register_dbus_path(path, id);
 
 	return 0;
@@ -1099,11 +1099,11 @@
 	char path[MAX_PATH_LENGTH];
 
 	if (unreg_dft) {
-		sprintf(path, "%s/%s/%s", MANAGER_PATH, HCI_DEFAULT_DEVICE_NAME, BLUEZ_HCI);
+		snprintf(path, sizeof(path), "%s/%s/%s", MANAGER_PATH, HCI_DEFAULT_DEVICE_NAME, BLUEZ_HCI);
 		unregister_dbus_path(path);
 	}
 
-	sprintf(path, "%s/%s%d/%s", MANAGER_PATH, HCI_DEVICE_NAME, id, BLUEZ_HCI);
+	snprintf(path, sizeof(path), "%s/%s%d/%s", MANAGER_PATH, HCI_DEVICE_NAME, id, BLUEZ_HCI);
 	unregister_dbus_path(path);
 
 	return ret;
@@ -1318,14 +1318,10 @@
 		syslog(LOG_ERR, "Can't send HCI commands:%s.", strerror(errno));
 		reply = bluez_new_failure_msg(msg, BLUEZ_ESYSTEM_OFFSET + errno);
 		goto failed;
-	} else {
-		uint8_t result = 0;
-		/* return TRUE to indicate that operation was completed */
-		reply = dbus_message_new_method_return(msg);
-		dbus_message_iter_init_append(reply, &iter);
-		dbus_message_iter_append_basic(&iter, DBUS_TYPE_BYTE ,&result);
 	}
 
+	reply = dbus_message_new_method_return(msg);
+
 failed:
 	if (dd >= 0)
 		close(dd);
@@ -1335,7 +1331,6 @@
 
 static DBusMessage* handle_cancel_periodic_inq_req(DBusMessage *msg, void *data)
 {
-	DBusMessageIter iter;
 	DBusMessage *reply = NULL;
 	struct hci_dbus_data *dbus_data = data;
 	int dd = -1;
@@ -1361,14 +1356,11 @@
 	if (hci_send_cmd(dd, OGF_LINK_CTL, OCF_EXIT_PERIODIC_INQUIRY, 0 , NULL) < 0) {
 		syslog(LOG_ERR, "Send hci command failed.");
 		reply = bluez_new_failure_msg(msg, BLUEZ_ESYSTEM_OFFSET + errno);
-	} else {
-		uint8_t result  = 0;
-		/* return TRUE to indicate that operation was completed */
-		reply = dbus_message_new_method_return(msg);
-		dbus_message_iter_init_append(reply, &iter);
-		dbus_message_iter_append_basic(&iter, DBUS_TYPE_BYTE ,&result);
+		goto failed;
 	}
 
+	reply = dbus_message_new_method_return(msg);
+
 failed:
 	if (dd >= 0)
 		close(dd);

^ permalink raw reply	[flat|nested] 27+ messages in thread
* [Bluez-devel] [PATCH] dbus.c cleanup
@ 2005-11-03  7:20 Johan Hedberg
  2005-11-03  8:34 ` Marcel Holtmann
  0 siblings, 1 reply; 27+ messages in thread
From: Johan Hedberg @ 2005-11-03  7:20 UTC (permalink / raw)
  To: bluez-devel

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

Hi Marcel,

Here's a cleanup patch for dbus.c which fixes some issues with
whitespace, coding-style, unecessary variables, etc.

I've also fixed PeriodicInquiry functionality of the python test script
(http://www.iki.fi/~jhedberg/bluez-python/pybt.py). It should now
support all currently implemented D-BUS services in hcid. Do you think
it would be worth including it in the bluez-utils cvs?

Johan

[-- Attachment #2: cleanup.patch --]
[-- Type: text/plain, Size: 5065 bytes --]

Index: hcid/dbus.c
===================================================================
RCS file: /cvsroot/bluez/utils/hcid/dbus.c,v
retrieving revision 1.44
diff -u -r1.44 dbus.c
--- hcid/dbus.c	2 Nov 2005 18:46:46 -0000	1.44
+++ hcid/dbus.c	3 Nov 2005 07:12:24 -0000
@@ -106,7 +106,7 @@
 typedef struct  {
 	uint32_t code;
 	const char *str;
-}bluez_error_t;
+} bluez_error_t;
 
 typedef struct {
 	char *str;
@@ -381,14 +381,15 @@
 static gboolean register_dbus_path(const char *path, uint16_t path_id, uint16_t dev_id,
 				const DBusObjectPathVTable *pvtable, gboolean fallback)
 {
-	struct hci_dbus_data *data;
+	gboolean ret = FALSE;
+	struct hci_dbus_data *data = NULL;
 
 	syslog(LOG_INFO,"Registering DBUS Path: %s", path);
 
 	data = malloc(sizeof(struct hci_dbus_data));
 	if (data == NULL) {
 		syslog(LOG_ERR,"Failed to alloc memory to DBUS path register data (%s)", path);
-		return FALSE;
+		goto out;
 	}
 
 	data->path_id = path_id;
@@ -396,19 +397,23 @@
 
 	if (fallback) {
 		if (!dbus_connection_register_fallback(connection, path, pvtable, data)) {
-			syslog(LOG_ERR,"DBUS failed to register %s object", path);
-			free(data);
-			return FALSE;
+			syslog(LOG_ERR,"DBUS failed to register %s fallback", path);
+			goto out;
 		}
 	} else {
 		if (!dbus_connection_register_object_path(connection, path, pvtable, data)) {
 			syslog(LOG_ERR,"DBUS failed to register %s object", path);
-			free(data);
-			return FALSE;
+			goto out;
 		}
 	}
 
-	return TRUE;
+	ret = TRUE;
+
+out:
+	if (!ret && data)
+		free(data);
+
+	return ret;
 }
 
 static gboolean unregister_dbus_path(const char *path)
@@ -468,8 +473,7 @@
 
 failed:
 	dbus_message_unref(message);
-	hci_send_cmd(dev, OGF_LINK_CTL,
-				OCF_PIN_CODE_NEG_REPLY, 6, &ci->bdaddr);
+	hci_send_cmd(dev, OGF_LINK_CTL, OCF_PIN_CODE_NEG_REPLY, 6, &ci->bdaddr);
 }
 
 void hcid_dbus_inquiry_start(bdaddr_t *local)
@@ -506,10 +510,7 @@
 
 failed:
 	dbus_message_unref(message);
-
 	bt_free(local_addr);
-
-	return;
 }
 
 void hcid_dbus_inquiry_complete(bdaddr_t *local)
@@ -546,10 +547,7 @@
 
 failed:
 	dbus_message_unref(message);
-
 	bt_free(local_addr);
-
-	return;
 }
 
 void hcid_dbus_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class, int8_t rssi)
@@ -598,8 +596,6 @@
 
 	bt_free(local_addr);
 	bt_free(peer_addr);
-
-	return;
 }
 
 void hcid_dbus_remote_name(bdaddr_t *local, bdaddr_t *peer, char *name)
@@ -645,8 +641,6 @@
 
 	bt_free(local_addr);
 	bt_free(peer_addr);
-
-	return;
 }
 
 void hcid_dbus_remote_name_failed(bdaddr_t *local, bdaddr_t *peer, uint8_t status)
@@ -692,8 +686,6 @@
 
 	bt_free(local_addr);
 	bt_free(peer_addr);
-
-	return;
 }
 
 void hcid_dbus_conn_complete(bdaddr_t *local, bdaddr_t *peer)
@@ -910,7 +902,6 @@
 
 	unregister_dbus_path(DEVICE_PATH);
 	unregister_dbus_path(MANAGER_PATH);
-
 }
 
 gboolean hcid_dbus_register_device(uint16_t id) 
@@ -1046,7 +1037,7 @@
 
 	for (; ptr->id != INVALID_PATH_ID; ptr++) {
 		if (ptr->unreg_func(connection, id) < 0)
-			goto failed;
+			syslog(LOG_ERR, "Unregistering profile id %04X failed", ptr->id);
 	}
 
 	up_adapters--;
@@ -1161,8 +1152,6 @@
 	const struct service_data *handlers = NULL;
 	DBusMessage *reply = NULL;
 	struct hci_dbus_data *dbus_data = data;
-	int type;
-	const char *iface;
 	const char *method;
 	const char *signature;
 	const char *path;
@@ -1170,8 +1159,6 @@
 	DBusHandlerResult ret = DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 
 	path = dbus_message_get_path(msg);
-	type = dbus_message_get_type(msg);
-	iface = dbus_message_get_interface(msg);
 	method = dbus_message_get_member(msg);
 	signature = dbus_message_get_signature(msg);
 
@@ -1216,11 +1203,9 @@
 	if (error)
 		reply = bluez_new_failure_msg(msg, error);
 
-
 	if (reply) {
-		if (!dbus_connection_send (conn, reply, NULL)) {
+		if (!dbus_connection_send (conn, reply, NULL))
 			syslog(LOG_ERR, "Can't send reply message!");
-		}
 		dbus_message_unref (reply);
 	}
 
@@ -1240,7 +1225,7 @@
 
 	path = dbus_message_get_path(msg);
 	iface = dbus_message_get_interface(msg);
-	method = dbus_message_get_member (msg);
+	method = dbus_message_get_member(msg);
 	signature = dbus_message_get_signature(msg);
 
 	syslog (LOG_INFO, "%s - path:%s", __PRETTY_FUNCTION__, path);
@@ -1264,11 +1249,9 @@
 	if (error)
 		reply = bluez_new_failure_msg(msg, error);
 
-
 	if (reply) {
-		if (!dbus_connection_send (conn, reply, NULL)) {
+		if (!dbus_connection_send (conn, reply, NULL))
 			syslog(LOG_ERR, "Can't send reply message!");
-		}
 		dbus_message_unref (reply);
 	}
 
@@ -1505,13 +1488,10 @@
 	if (hci_switch_role(dd, &bdaddr, role, 10000) < 0) {
 		syslog(LOG_ERR, "Switch role request failed\n");
 		reply = bluez_new_failure_msg(msg, BLUEZ_ESYSTEM_OFFSET + errno);
-	} else {
-		uint8_t result = 0;
-		/* return TRUE to indicate that operation was completed */
-		reply = dbus_message_new_method_return(msg);
-		dbus_message_iter_init_append(reply, &iter);
-		dbus_message_iter_append_basic(&iter, DBUS_TYPE_BYTE, &result);
+		goto failed;
 	}
+	
+	reply = dbus_message_new_method_return(msg);
 
 failed:
 	return reply;

^ permalink raw reply	[flat|nested] 27+ messages in thread
end of thread, other threads:[~2005-11-03 14:36 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-10-29 17:53 [Bluez-devel] [PATCH] dbus.c cleanup Johan Hedberg
2005-10-29 18:07 ` Marcel Holtmann
2005-10-29 18:48   ` Johan Hedberg
2005-10-29 19:07     ` Marcel Holtmann
2005-10-31 11:59       ` Claudio Takahasi
2005-10-31 12:07         ` Johan Hedberg
2005-10-31 12:14           ` Marcel Holtmann
2005-10-31 12:30             ` Eduardo Rocha
2005-10-31 12:12         ` Marcel Holtmann
2005-10-31 13:47           ` Claudio Takahasi
2005-11-01 13:11             ` Marcel Holtmann
2005-11-01 13:21               ` Claudio Takahasi
2005-11-01 19:08                 ` Claudio Takahasi
2005-11-01 19:58                   ` Marcel Holtmann
2005-11-01 20:23                     ` Johan Hedberg
2005-11-01 20:40                       ` Marcel Holtmann
2005-11-01 20:53                       ` Claudio Takahasi
  -- strict thread matches above, loose matches on Subject: below --
2005-11-03  7:20 Johan Hedberg
2005-11-03  8:34 ` Marcel Holtmann
2005-11-03  8:54   ` Johan Hedberg
2005-11-03  9:08     ` Marcel Holtmann
2005-11-03  9:23       ` Johan Hedberg
2005-11-03  9:41         ` Marcel Holtmann
2005-11-03 10:13           ` Johan Hedberg
2005-11-03 10:33             ` Marcel Holtmann
2005-11-03 14:22               ` Johan Hedberg
2005-11-03 14:36                 ` Marcel Holtmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).