Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH 3/3] More CodingStyle in lib and tools
From: Michal Labedzki @ 2010-12-20 10:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Michal Labedzki
In-Reply-To: <1292840029-27770-1-git-send-email-michal.labedzki@tieto.com>

---
 lib/hci.c         |   69 +++++++++++++++++++++++++++-------------
 tools/hciconfig.c |   91 +++++++++++++++++++++++++++++++++-------------------
 tools/hcitool.c   |   20 ++++++------
 3 files changed, 114 insertions(+), 66 deletions(-)

diff --git a/lib/hci.c b/lib/hci.c
index 64c7dad..9e53479 100644
--- a/lib/hci.c
+++ b/lib/hci.c
@@ -56,8 +56,8 @@ typedef struct {
 
 static int is_number(const char *c)
 {
-	while(*c) {
-		if (! isdigit(*c))
+	while (*c) {
+		if (!isdigit(*c))
 			return 0;
 		++c;
 	}
@@ -138,7 +138,8 @@ static int hci_str2uint(hci_map *map, char *str, unsigned int *val)
 	while ((t = strsep(&ptr, ","))) {
 		for (m = map; m->str; m++) {
 			if (!strcasecmp(m->str,t)) {
-				*val = (unsigned int) m->val; set = 1;
+				*val = (unsigned int) m->val;
+				set = 1;
 				break;
 			}
 		}
@@ -781,7 +782,8 @@ char *lmp_featurestostr(uint8_t *features, char *pref, int width)
 
 		while (m->str) {
 			if (m->val & features[i])
-				size += strlen(m->str) + (pref ? strlen(pref) : 0) + 1;
+				size += strlen(m->str) +
+						(pref ? strlen(pref) : 0) + 1;
 			m++;
 		}
 	}
@@ -803,7 +805,8 @@ char *lmp_featurestostr(uint8_t *features, char *pref, int width)
 		while (m->str) {
 			if (m->val & features[i]) {
 				if (strlen(off) + strlen(m->str) > maxwidth) {
-					ptr += sprintf(ptr, "\n%s", pref ? pref : "");
+					ptr += sprintf(ptr, "\n%s",
+							pref ? pref : "");
 					off = ptr;
 				}
 				ptr += sprintf(ptr, "%s ", m->str);
@@ -816,8 +819,8 @@ char *lmp_featurestostr(uint8_t *features, char *pref, int width)
 }
 
 /* HCI functions that do not require open device */
-
-int hci_for_each_dev(int flag, int (*func)(int dd, int dev_id, long arg), long arg)
+int hci_for_each_dev(int flag, int (*func)(int dd, int dev_id, long arg),
+			long arg)
 {
 	struct hci_dev_list_req *dl;
 	struct hci_dev_req *dr;
@@ -951,7 +954,8 @@ int hci_devba(int dev_id, bdaddr_t *bdaddr)
 	return 0;
 }
 
-int hci_inquiry(int dev_id, int len, int nrsp, const uint8_t *lap, inquiry_info **ii, long flags)
+int hci_inquiry(int dev_id, int len, int nrsp, const uint8_t *lap,
+		inquiry_info **ii, long flags)
 {
 	struct hci_inquiry_req *ir;
 	uint8_t num_rsp = nrsp;
@@ -1138,7 +1142,8 @@ int hci_send_req(int dd, struct hci_request *r, int to)
 			}
 
 			to -= 10;
-			if (to < 0) to = 0;
+			if (to < 0)
+				to = 0;
 
 		}
 
@@ -1231,7 +1236,9 @@ done:
 	return 0;
 }
 
-int hci_create_connection(int dd, const bdaddr_t *bdaddr, uint16_t ptype, uint16_t clkoffset, uint8_t rswitch, uint16_t *handle, int to)
+int hci_create_connection(int dd, const bdaddr_t *bdaddr, uint16_t ptype,
+				uint16_t clkoffset, uint8_t rswitch,
+				uint16_t *handle, int to)
 {
 	evt_conn_complete rp;
 	create_conn_cp cp;
@@ -1338,7 +1345,10 @@ int hci_write_local_name(int dd, const char *name, int to)
 	return 0;
 }
 
-int hci_read_remote_name_with_clock_offset(int dd, const bdaddr_t *bdaddr, uint8_t pscan_rep_mode, uint16_t clkoffset, int len, char *name, int to)
+int hci_read_remote_name_with_clock_offset(int dd, const bdaddr_t *bdaddr,
+						uint8_t pscan_rep_mode,
+						uint16_t clkoffset,
+						int len, char *name, int to)
 {
 	evt_remote_name_req_complete rn;
 	remote_name_req_cp cp;
@@ -1371,9 +1381,11 @@ int hci_read_remote_name_with_clock_offset(int dd, const bdaddr_t *bdaddr, uint8
 	return 0;
 }
 
-int hci_read_remote_name(int dd, const bdaddr_t *bdaddr, int len, char *name, int to)
+int hci_read_remote_name(int dd, const bdaddr_t *bdaddr, int len, char *name,
+				int to)
 {
-	return hci_read_remote_name_with_clock_offset(dd, bdaddr, 0x02, 0x0000, len, name, to);
+	return hci_read_remote_name_with_clock_offset(dd, bdaddr, 0x02, 0x0000,
+							len, name, to);
 }
 
 int hci_read_remote_name_cancel(int dd, const bdaddr_t *bdaddr, int to)
@@ -1396,7 +1408,8 @@ int hci_read_remote_name_cancel(int dd, const bdaddr_t *bdaddr, int to)
 	return 0;
 }
 
-int hci_read_remote_version(int dd, uint16_t handle, struct hci_version *ver, int to)
+int hci_read_remote_version(int dd, uint16_t handle, struct hci_version *ver,
+				int to)
 {
 	evt_read_remote_version_complete rp;
 	read_remote_version_cp cp;
@@ -1460,7 +1473,9 @@ int hci_read_remote_features(int dd, uint16_t handle, uint8_t *features, int to)
 	return 0;
 }
 
-int hci_read_remote_ext_features(int dd, uint16_t handle, uint8_t page, uint8_t *max_page, uint8_t *features, int to)
+int hci_read_remote_ext_features(int dd, uint16_t handle, uint8_t page,
+					uint8_t *max_page, uint8_t *features,
+					int to)
 {
 	evt_read_remote_ext_features_complete rp;
 	read_remote_ext_features_cp cp;
@@ -1603,7 +1618,8 @@ int hci_read_local_features(int dd, uint8_t *features, int to)
 	return 0;
 }
 
-int hci_read_local_ext_features(int dd, uint8_t page, uint8_t *max_page, uint8_t *features, int to)
+int hci_read_local_ext_features(int dd, uint8_t page, uint8_t *max_page,
+				uint8_t *features, int to)
 {
 	read_local_ext_features_cp cp;
 	read_local_ext_features_rp rp;
@@ -1944,7 +1960,8 @@ int hci_switch_role(int dd, bdaddr_t *bdaddr, uint8_t role, int to)
 	return 0;
 }
 
-int hci_park_mode(int dd, uint16_t handle, uint16_t max_interval, uint16_t min_interval, int to)
+int hci_park_mode(int dd, uint16_t handle, uint16_t max_interval,
+			uint16_t min_interval, int to)
 {
 	park_mode_cp cp;
 	evt_mode_change rp;
@@ -2342,7 +2359,8 @@ int hci_write_inquiry_transmit_power_level(int dd, int8_t level, int to)
 	return 0;
 }
 
-int hci_read_transmit_power_level(int dd, uint16_t handle, uint8_t type, int8_t *level, int to)
+int hci_read_transmit_power_level(int dd, uint16_t handle, uint8_t type,
+					int8_t *level, int to)
 {
 	read_transmit_power_level_cp cp;
 	read_transmit_power_level_rp rp;
@@ -2426,7 +2444,8 @@ int hci_write_link_policy(int dd, uint16_t handle, uint16_t policy, int to)
 	return 0;
 }
 
-int hci_read_link_supervision_timeout(int dd, uint16_t handle, uint16_t *timeout, int to)
+int hci_read_link_supervision_timeout(int dd, uint16_t handle,
+					uint16_t *timeout, int to)
 {
 	read_link_supervision_timeout_rp rp;
 	struct hci_request rq;
@@ -2451,7 +2470,8 @@ int hci_read_link_supervision_timeout(int dd, uint16_t handle, uint16_t *timeout
 	return 0;
 }
 
-int hci_write_link_supervision_timeout(int dd, uint16_t handle, uint16_t timeout, int to)
+int hci_write_link_supervision_timeout(int dd, uint16_t handle,
+					uint16_t timeout, int to)
 {
 	write_link_supervision_timeout_cp cp;
 	write_link_supervision_timeout_rp rp;
@@ -2508,7 +2528,8 @@ int hci_set_afh_classification(int dd, uint8_t *map, int to)
 	return 0;
 }
 
-int hci_read_link_quality(int dd, uint16_t handle, uint8_t *link_quality, int to)
+int hci_read_link_quality(int dd, uint16_t handle, uint8_t *link_quality,
+				int to)
 {
 	read_link_quality_rp rp;
 	struct hci_request rq;
@@ -2558,7 +2579,8 @@ int hci_read_rssi(int dd, uint16_t handle, int8_t *rssi, int to)
 	return 0;
 }
 
-int hci_read_afh_map(int dd, uint16_t handle, uint8_t *mode, uint8_t *map, int to)
+int hci_read_afh_map(int dd, uint16_t handle, uint8_t *mode, uint8_t *map,
+			int to)
 {
 	read_afh_map_rp rp;
 	struct hci_request rq;
@@ -2584,7 +2606,8 @@ int hci_read_afh_map(int dd, uint16_t handle, uint8_t *mode, uint8_t *map, int t
 	return 0;
 }
 
-int hci_read_clock(int dd, uint16_t handle, uint8_t which, uint32_t *clock, uint16_t *accuracy, int to)
+int hci_read_clock(int dd, uint16_t handle, uint8_t which, uint32_t *clock,
+			uint16_t *accuracy, int to)
 {
 	read_clock_cp cp;
 	read_clock_rp rp;
diff --git a/tools/hciconfig.c b/tools/hciconfig.c
index 0a3668e..6eadd3e 100644
--- a/tools/hciconfig.c
+++ b/tools/hciconfig.c
@@ -57,7 +57,8 @@ static void print_dev_list(int ctl, int flags)
 	struct hci_dev_req *dr;
 	int i;
 
-	if (!(dl = malloc(HCI_MAX_DEV * sizeof(struct hci_dev_req) + sizeof(uint16_t)))) {
+	if (!(dl = malloc(HCI_MAX_DEV * sizeof(struct hci_dev_req) +
+		sizeof(uint16_t)))) {
 		perror("Can't allocate memory");
 		exit(1);
 	}
@@ -501,7 +502,7 @@ static char *get_minor_device_name(int major, int minor)
 	case 0:	/* misc */
 		return "";
 	case 1:	/* computer */
-		switch(minor) {
+		switch (minor) {
 		case 0:
 			return "Uncategorized";
 		case 1:
@@ -519,7 +520,7 @@ static char *get_minor_device_name(int major, int minor)
 		}
 		break;
 	case 2:	/* phone */
-		switch(minor) {
+		switch (minor) {
 		case 0:
 			return "Uncategorized";
 		case 1:
@@ -539,7 +540,7 @@ static char *get_minor_device_name(int major, int minor)
 	case 3:	/* lan access */
 		if (minor == 0)
 			return "Uncategorized";
-		switch(minor / 8) {
+		switch (minor / 8) {
 		case 0:
 			return "Fully available";
 		case 1:
@@ -559,7 +560,7 @@ static char *get_minor_device_name(int major, int minor)
 		}
 		break;
 	case 4:	/* audio/video */
-		switch(minor) {
+		switch (minor) {
 		case 0:
 			return "Uncategorized";
 		case 1:
@@ -603,7 +604,7 @@ static char *get_minor_device_name(int major, int minor)
 
 		cls_str[0] = '\0';
 
-		switch(minor & 48) {
+		switch (minor & 48) {
 		case 16:
 			strncpy(cls_str, "Keyboard", sizeof(cls_str));
 			break;
@@ -614,10 +615,10 @@ static char *get_minor_device_name(int major, int minor)
 			strncpy(cls_str, "Combo keyboard/pointing device", sizeof(cls_str));
 			break;
 		}
-		if((minor & 15) && (strlen(cls_str) > 0))
+		if ((minor & 15) && (strlen(cls_str) > 0))
 			strcat(cls_str, "/");
 
-		switch(minor & 15) {
+		switch (minor & 15) {
 		case 0:
 			break;
 		case 1:
@@ -642,7 +643,7 @@ static char *get_minor_device_name(int major, int minor)
 			strncat(cls_str, "(reserved)", sizeof(cls_str) - strlen(cls_str));
 			break;
 		}
-		if(strlen(cls_str) > 0)
+		if (strlen(cls_str) > 0)
 			return cls_str;
 	}
 	case 6:	/* imaging */
@@ -656,7 +657,7 @@ static char *get_minor_device_name(int major, int minor)
 			return "Printer";
 		break;
 	case 7: /* wearable */
-		switch(minor) {
+		switch (minor) {
 		case 1:
 			return "Wrist Watch";
 		case 2:
@@ -670,7 +671,7 @@ static char *get_minor_device_name(int major, int minor)
 		}
 		break;
 	case 8: /* toy */
-		switch(minor) {
+		switch (minor) {
 		case 1:
 			return "Robot";
 		case 2:
@@ -754,10 +755,24 @@ static void cmd_class(int ctl, int hdev, char *opt)
 
 static void cmd_voice(int ctl, int hdev, char *opt)
 {
-	static char *icf[] = { "Linear", "u-Law", "A-Law", "Reserved" };
-	static char *idf[] = { "1's complement", "2's complement", "Sign-Magnitude", "Reserved" };
-	static char *iss[] = { "8 bit", "16 bit" };
-	static char *acf[] = { "CVSD", "u-Law", "A-Law", "Reserved" };
+	static char *icf[] = {	"Linear",
+				"u-Law",
+				"A-Law",
+				"Reserved" };
+
+	static char *idf[] = {	"1's complement",
+				"2's complement",
+				"Sign-Magnitude",
+				"Reserved" };
+
+	static char *iss[] = {	"8 bit",
+				"16 bit" };
+
+	static char *acf[] = {	"CVSD",
+				"u-Law",
+				"A-Law",
+				"Reserved" };
+
 	int s = hci_open_dev(hdev);
 
 	if (s < 0) {
@@ -787,15 +802,19 @@ static void cmd_voice(int ctl, int hdev, char *opt)
 			((vs & 0x03fc) == 0x0060) ? " (Default Condition)" : "");
 		printf("\tInput Coding: %s\n", icf[ic]);
 		printf("\tInput Data Format: %s\n", idf[(vs & 0xc0) >> 6]);
+
 		if (!ic) {
-			printf("\tInput Sample Size: %s\n", iss[(vs & 0x20) >> 5]);
-			printf("\t# of bits padding at MSB: %d\n", (vs & 0x1c) >> 2);
+			printf("\tInput Sample Size: %s\n",
+				iss[(vs & 0x20) >> 5]);
+			printf("\t# of bits padding at MSB: %d\n",
+				(vs & 0x1c) >> 2);
 		}
 		printf("\tAir Coding Format: %s\n", acf[vs & 0x03]);
 	}
 }
 
-static int get_link_key(const bdaddr_t *local, const bdaddr_t *peer, uint8_t *key)
+static int get_link_key(const bdaddr_t *local, const bdaddr_t *peer,
+			uint8_t *key)
 {
 	char filename[PATH_MAX + 1], addr[18], tmp[3], *str;
 	int i;
@@ -1356,8 +1375,10 @@ static void cmd_page_parms(int ctl, int hdev, char *opt)
 
 		window   = btohs(rp.window);
 		interval = btohs(rp.interval);
-		printf("\tPage interval: %u slots (%.2f ms), window: %u slots (%.2f ms)\n",
-				interval, (float)interval * 0.625, window, (float)window * 0.625);
+		printf("\tPage interval: %u slots (%.2f ms), "
+			"window: %u slots (%.2f ms)\n",
+			interval, (float)interval * 0.625,
+			window, (float)window * 0.625);
 	}
 }
 
@@ -1441,7 +1462,7 @@ static void cmd_afh_mode(int ctl, int hdev, char *opt)
 
 		if (hci_write_afh_mode(dd, mode, 2000) < 0) {
 			fprintf(stderr, "Can't set AFH mode on hci%d: %s (%d)\n",
-						hdev, strerror(errno), errno);
+					hdev, strerror(errno), errno);
 			exit(1);
 		}
 	} else {
@@ -1449,7 +1470,7 @@ static void cmd_afh_mode(int ctl, int hdev, char *opt)
 
 		if (hci_read_afh_mode(dd, &mode, 1000) < 0) {
 			fprintf(stderr, "Can't read AFH mode on hci%d: %s (%d)\n",
-						hdev, strerror(errno), errno);
+					hdev, strerror(errno), errno);
 			exit(1);
 		}
 
@@ -1474,7 +1495,7 @@ static void cmd_ssp_mode(int ctl, int hdev, char *opt)
 
 		if (hci_write_simple_pairing_mode(dd, mode, 2000) < 0) {
 			fprintf(stderr, "Can't set Simple Pairing mode on hci%d: %s (%d)\n",
-						hdev, strerror(errno), errno);
+					hdev, strerror(errno), errno);
 			exit(1);
 		}
 	} else {
@@ -1482,12 +1503,13 @@ static void cmd_ssp_mode(int ctl, int hdev, char *opt)
 
 		if (hci_read_simple_pairing_mode(dd, &mode, 1000) < 0) {
 			fprintf(stderr, "Can't read Simple Pairing mode on hci%d: %s (%d)\n",
-						hdev, strerror(errno), errno);
+					hdev, strerror(errno), errno);
 			exit(1);
 		}
 
 		print_dev_hdr(&di);
-		printf("\tSimple Pairing mode: %s\n", mode == 1 ? "Enabled" : "Disabled");
+		printf("\tSimple Pairing mode: %s\n",
+			mode == 1 ? "Enabled" : "Disabled");
 	}
 }
 
@@ -1505,7 +1527,8 @@ static void print_rev_ericsson(int dd)
 	rq.rlen   = sizeof(buf);
 
 	if (hci_send_req(dd, &rq, 1000) < 0) {
-		printf("\nCan't read revision info: %s (%d)\n", strerror(errno), errno);
+		printf("\nCan't read revision info: %s (%d)\n",
+			strerror(errno), errno);
 		return;
 	}
 
@@ -1551,7 +1574,8 @@ static void print_rev_digianswer(int dd)
 	rq.rlen   = sizeof(buf);
 
 	if (hci_send_req(dd, &rq, 1000) < 0) {
-		printf("\nCan't read revision info: %s (%d)\n", strerror(errno), errno);
+		printf("\nCan't read revision info: %s (%d)\n",
+			strerror(errno), errno);
 		return;
 	}
 
@@ -1560,7 +1584,8 @@ static void print_rev_digianswer(int dd)
 
 static void print_rev_broadcom(uint16_t hci_rev, uint16_t lmp_subver)
 {
-	printf("\tFirmware %d.%d / %d\n", hci_rev & 0xff, lmp_subver >> 8, lmp_subver & 0xff);
+	printf("\tFirmware %d.%d / %d\n",
+		hci_rev & 0xff, lmp_subver >> 8, lmp_subver & 0xff);
 }
 
 static void print_rev_avm(uint16_t hci_rev, uint16_t lmp_subver)
@@ -1723,7 +1748,7 @@ static void print_dev_info(int ctl, struct hci_dev_info *di)
 
 static int is_number(const char *c)
 {
-	while(*c) {
+	while (*c) {
 		if (! isdigit(*c))
 			return 0;
 		++c;
@@ -1807,7 +1832,7 @@ static void usage(void)
 		"\thciconfig\n"
 		"\thciconfig [-a] hciX [command ...]\n");
 	printf("Commands:\n");
-	for (i=0; command[i].cmd; i++)
+	for (i = 0; command[i].cmd; i++)
 		printf("\t%-10s %-8s\t%s\n", command[i].cmd,
 		command[i].opt ? command[i].opt : " ",
 		command[i].doc);
@@ -1821,10 +1846,10 @@ static struct option main_options[] = {
 
 int main(int argc, char *argv[])
 {
-	int opt, ctl, i, cmd=0;
+	int opt, ctl, i, cmd = 0;
 
-	while ((opt=getopt_long(argc, argv, "ah", main_options, NULL)) != -1) {
-		switch(opt) {
+	while ((opt = getopt_long(argc, argv, "ah", main_options, NULL)) != -1) {
+		switch (opt) {
 		case 'a':
 			all = 1;
 			break;
diff --git a/tools/hcitool.c b/tools/hcitool.c
index 847bf1b..d50adaf 100644
--- a/tools/hcitool.c
+++ b/tools/hcitool.c
@@ -197,7 +197,7 @@ static char *get_minor_device_name(int major, int minor)
 	case 0:	/* misc */
 		return "";
 	case 1:	/* computer */
-		switch(minor) {
+		switch (minor) {
 		case 0:
 			return "Uncategorized";
 		case 1:
@@ -215,7 +215,7 @@ static char *get_minor_device_name(int major, int minor)
 		}
 		break;
 	case 2:	/* phone */
-		switch(minor) {
+		switch (minor) {
 		case 0:
 			return "Uncategorized";
 		case 1:
@@ -235,7 +235,7 @@ static char *get_minor_device_name(int major, int minor)
 	case 3:	/* lan access */
 		if (minor == 0)
 			return "Uncategorized";
-		switch(minor / 8) {
+		switch (minor / 8) {
 		case 0:
 			return "Fully available";
 		case 1:
@@ -255,7 +255,7 @@ static char *get_minor_device_name(int major, int minor)
 		}
 		break;
 	case 4:	/* audio/video */
-		switch(minor) {
+		switch (minor) {
 		case 0:
 			return "Uncategorized";
 		case 1:
@@ -297,7 +297,7 @@ static char *get_minor_device_name(int major, int minor)
 	case 5:	/* peripheral */ {
 		static char cls_str[48]; cls_str[0] = 0;
 
-		switch(minor & 48) {
+		switch (minor & 48) {
 		case 16:
 			strncpy(cls_str, "Keyboard", sizeof(cls_str));
 			break;
@@ -308,10 +308,10 @@ static char *get_minor_device_name(int major, int minor)
 			strncpy(cls_str, "Combo keyboard/pointing device", sizeof(cls_str));
 			break;
 		}
-		if((minor & 15) && (strlen(cls_str) > 0))
+		if ((minor & 15) && (strlen(cls_str) > 0))
 			strcat(cls_str, "/");
 
-		switch(minor & 15) {
+		switch (minor & 15) {
 		case 0:
 			break;
 		case 1:
@@ -336,7 +336,7 @@ static char *get_minor_device_name(int major, int minor)
 			strncat(cls_str, "(reserved)", sizeof(cls_str) - strlen(cls_str));
 			break;
 		}
-		if(strlen(cls_str) > 0)
+		if (strlen(cls_str) > 0)
 			return cls_str;
 	}
 	case 6:	/* imaging */
@@ -350,7 +350,7 @@ static char *get_minor_device_name(int major, int minor)
 			return "Printer";
 		break;
 	case 7: /* wearable */
-		switch(minor) {
+		switch (minor) {
 		case 1:
 			return "Wrist Watch";
 		case 2:
@@ -364,7 +364,7 @@ static char *get_minor_device_name(int major, int minor)
 		}
 		break;
 	case 8: /* toy */
-		switch(minor) {
+		switch (minor) {
 		case 1:
 			return "Robot";
 		case 2:
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 2/3] Fix tools UI to avoid program launch mistakes.
From: Michal Labedzki @ 2010-12-20 10:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Michal Labedzki
In-Reply-To: <1292840029-27770-1-git-send-email-michal.labedzki@tieto.com>

hciconfig: warrning user on unknown commands
hcitool: return error on unknown command in hcitool
fix length size in comparision to avoid ambiguity commands
check if command number of arguments is correct
---
 tools/hciconfig.c |   10 ++-
 tools/hcitool.c   |  201 ++++++++++++++++------------------------------------
 2 files changed, 70 insertions(+), 141 deletions(-)

diff --git a/tools/hciconfig.c b/tools/hciconfig.c
index e8b0c69..0a3668e 100644
--- a/tools/hciconfig.c
+++ b/tools/hciconfig.c
@@ -1805,7 +1805,7 @@ static void usage(void)
 	printf("hciconfig - HCI device configuration utility\n");
 	printf("Usage:\n"
 		"\thciconfig\n"
-		"\thciconfig [-a] hciX [command]\n");
+		"\thciconfig [-a] hciX [command ...]\n");
 	printf("Commands:\n");
 	for (i=0; command[i].cmd; i++)
 		printf("\t%-10s %-8s\t%s\n", command[i].cmd,
@@ -1869,7 +1869,8 @@ int main(int argc, char *argv[])
 
 	while (argc > 0) {
 		for (i = 0; command[i].cmd; i++) {
-			if (strncmp(command[i].cmd, *argv, 5))
+			if (strncmp(command[i].cmd,
+					*argv, strlen(command[i].cmd)))
 				continue;
 
 			if (command[i].opt) {
@@ -1880,6 +1881,11 @@ int main(int argc, char *argv[])
 			cmd = 1;
 			break;
 		}
+
+		if (command[i].cmd == 0)
+			fprintf(stderr, "Warning: unknown command - \"%s\"\n",
+					*argv);
+
 		argc--; argv++;
 	}
 
diff --git a/tools/hcitool.c b/tools/hcitool.c
index dace674..847bf1b 100644
--- a/tools/hcitool.c
+++ b/tools/hcitool.c
@@ -66,6 +66,31 @@ static int dev_info(int s, int dev_id, long arg)
 	return 0;
 }
 
+static void helper_arg(int min_num_arg, int max_num_arg, int *argc,
+			char ***argv, const char *usage)
+{
+	*argc -= optind;
+	/* too many arguments, but when "max_num_arg < min_num_arg" then no
+		 limiting (prefer "max_num_arg=-1" to gen infinity)
+	*/
+	if ( (*argc > max_num_arg) && (max_num_arg >= min_num_arg ) ) {
+		fprintf(stderr, "%s: too many arguments (maximal: %i)\n",
+				*argv[0], max_num_arg);
+		printf("%s", usage);
+		exit(1);
+	}
+
+	/* print usage */
+	if (*argc < min_num_arg) {
+		fprintf(stderr, "%s: too few arguments (minimal: %i)\n",
+				*argv[0], min_num_arg);
+		printf("%s", usage);
+		exit(0);
+	}
+
+	*argv += optind;
+}
+
 static char *type2str(uint8_t type)
 {
 	switch (type) {
@@ -396,6 +421,7 @@ static void cmd_dev(int dev_id, int argc, char **argv)
 			return;
 		}
 	}
+	helper_arg(0, 0, &argc, &argv, dev_help);
 
 	printf("Devices:\n");
 
@@ -466,6 +492,7 @@ static void cmd_inq(int dev_id, int argc, char **argv)
 			return;
 		}
 	}
+	helper_arg(0, 0, &argc, &argv, inq_help);
 
 	printf("Inquiring ...\n");
 
@@ -581,6 +608,7 @@ static void cmd_scan(int dev_id, int argc, char **argv)
 			return;
 		}
 	}
+	helper_arg(0, 0, &argc, &argv, scan_help);
 
 	if (dev_id < 0) {
 		dev_id = hci_get_route(NULL);
@@ -790,13 +818,7 @@ static void cmd_name(int dev_id, int argc, char **argv)
 			return;
 		}
 	}
-	argc -= optind;
-	argv += optind;
-
-	if (argc < 1) {
-		printf("%s", name_help);
-		return;
-	}
+	helper_arg(1, 1, &argc, &argv, name_help);
 
 	str2ba(argv[0], &bdaddr);
 
@@ -849,13 +871,7 @@ static void cmd_info(int dev_id, int argc, char **argv)
 			return;
 		}
 	}
-	argc -= optind;
-	argv += optind;
-
-	if (argc < 1) {
-		printf("%s", info_help);
-		return;
-	}
+	helper_arg(1, 1, &argc, &argv, info_help);
 
 	str2ba(argv[0], &bdaddr);
 
@@ -991,6 +1007,7 @@ static void cmd_spinq(int dev_id, int argc, char **argv)
 			return;
 		}
 	}
+	helper_arg(0, 0, &argc, &argv, spinq_help);
 
 	if (dev_id < 0)
 		dev_id = hci_get_route(NULL);
@@ -1031,7 +1048,7 @@ static struct option epinq_options[] = {
 
 static const char *epinq_help =
 	"Usage:\n"
-	"\tspinq\n";
+	"\tepinq\n";
 
 static void cmd_epinq(int dev_id, int argc, char **argv)
 {
@@ -1044,6 +1061,7 @@ static void cmd_epinq(int dev_id, int argc, char **argv)
 			return;
 		}
 	}
+	helper_arg(0, 0, &argc, &argv, epinq_help);
 
 	if (dev_id < 0)
 		dev_id = hci_get_route(NULL);
@@ -1092,13 +1110,7 @@ static void cmd_cmd(int dev_id, int argc, char **argv)
 			return;
 		}
 	}
-	argc -= optind;
-	argv += optind;
-
-	if (argc < 2) {
-		printf("%s", cmd_help);
-		return;
-	}
+	helper_arg(2, -1, &argc, &argv, cmd_help);
 
 	if (dev_id < 0)
 		dev_id = hci_get_route(NULL);
@@ -1175,6 +1187,7 @@ static void cmd_con(int dev_id, int argc, char **argv)
 			return;
 		}
 	}
+	helper_arg(0, 0, &argc, &argv, con_help);
 
 	printf("Connections:\n");
 
@@ -1223,13 +1236,7 @@ static void cmd_cc(int dev_id, int argc, char **argv)
 			return;
 		}
 	}
-	argc -= optind;
-	argv += optind;
-
-	if (argc < 1) {
-		printf("%s", cc_help);
-		return;
-	}
+	helper_arg(1, 1, &argc, &argv, cc_help);
 
 	str2ba(argv[0], &bdaddr);
 
@@ -1279,13 +1286,7 @@ static void cmd_dc(int dev_id, int argc, char **argv)
 			return;
 		}
 	}
-	argc -= optind;
-	argv += optind;
-
-	if (argc < 1) {
-		printf("%s", dc_help);
-		return;
-	}
+	helper_arg(1, 2, &argc, &argv, dc_help);
 
 	str2ba(argv[0], &bdaddr);
 	reason = (argc > 1) ? atoi(argv[1]) : HCI_OE_USER_ENDED_CONNECTION;
@@ -1350,13 +1351,7 @@ static void cmd_sr(int dev_id, int argc, char **argv)
 			return;
 		}
 	}
-	argc -= optind;
-	argv += optind;
-
-	if (argc < 2) {
-		printf("%s", sr_help);
-		return;
-	}
+	helper_arg(2, 2, &argc, &argv, sr_help);
 
 	str2ba(argv[0], &bdaddr);
 	switch (argv[1][0]) {
@@ -1418,13 +1413,7 @@ static void cmd_rssi(int dev_id, int argc, char **argv)
 			return;
 		}
 	}
-	argc -= optind;
-	argv += optind;
-
-	if (argc < 1) {
-		printf("%s", rssi_help);
-		return;
-	}
+	helper_arg(1, 1, &argc, &argv, rssi_help);
 
 	str2ba(argv[0], &bdaddr);
 
@@ -1492,13 +1481,7 @@ static void cmd_lq(int dev_id, int argc, char **argv)
 			return;
 		}
 	}
-	argc -= optind;
-	argv += optind;
-
-	if (argc < 1) {
-		printf("%s", lq_help);
-		return;
-	}
+	helper_arg(1, 1, &argc, &argv, lq_help);
 
 	str2ba(argv[0], &bdaddr);
 
@@ -1567,13 +1550,7 @@ static void cmd_tpl(int dev_id, int argc, char **argv)
 			return;
 		}
 	}
-	argc -= optind;
-	argv += optind;
-
-	if (argc < 1) {
-		printf("%s", tpl_help);
-		return;
-	}
+	helper_arg(1, 2, &argc, &argv, tpl_help);
 
 	str2ba(argv[0], &bdaddr);
 	type = (argc > 1) ? atoi(argv[1]) : 0;
@@ -1644,13 +1621,7 @@ static void cmd_afh(int dev_id, int argc, char **argv)
 			return;
 		}
 	}
-	argc -= optind;
-	argv += optind;
-
-	if (argc < 1) {
-		printf("%s", afh_help);
-		return;
-	}
+	helper_arg(1, 1, &argc, &argv, afh_help);
 
 	str2ba(argv[0], &bdaddr);
 
@@ -1730,13 +1701,7 @@ static void cmd_cpt(int dev_id, int argc, char **argv)
 			return;
 		}
 	}
-	argc -= optind;
-	argv += optind;
-
-	if (argc < 2) {
-		printf("%s", cpt_help);
-		return;
-	}
+	helper_arg(2, 2, &argc, &argv, cpt_help);
 
 	str2ba(argv[0], &bdaddr);
 	hci_strtoptype(argv[1], &ptype);
@@ -1815,13 +1780,7 @@ static void cmd_lp(int dev_id, int argc, char **argv)
 			return;
 		}
 	}
-	argc -= optind;
-	argv += optind;
-
-	if (argc < 1) {
-		printf("%s", lp_help);
-		return;
-	}
+	helper_arg(1, 2, &argc, &argv, lp_help);
 
 	str2ba(argv[0], &bdaddr);
 
@@ -1914,13 +1873,7 @@ static void cmd_lst(int dev_id, int argc, char **argv)
 			return;
 		}
 	}
-	argc -= optind;
-	argv += optind;
-
-	if (argc < 1) {
-		printf("%s", lst_help);
-		return;
-	}
+	helper_arg(1, 2, &argc, &argv, lst_help);
 
 	str2ba(argv[0], &bdaddr);
 
@@ -2004,13 +1957,7 @@ static void cmd_auth(int dev_id, int argc, char **argv)
 			return;
 		}
 	}
-	argc -= optind;
-	argv += optind;
-
-	if (argc < 1) {
-		printf("%s", auth_help);
-		return;
-	}
+	helper_arg(1, 1, &argc, &argv, auth_help);
 
 	str2ba(argv[0], &bdaddr);
 
@@ -2076,13 +2023,7 @@ static void cmd_enc(int dev_id, int argc, char **argv)
 			return;
 		}
 	}
-	argc -= optind;
-	argv += optind;
-
-	if (argc < 1) {
-		printf("%s", enc_help);
-		return;
-	}
+	helper_arg(1, 2, &argc, &argv, enc_help);
 
 	str2ba(argv[0], &bdaddr);
 
@@ -2149,13 +2090,7 @@ static void cmd_key(int dev_id, int argc, char **argv)
 			return;
 		}
 	}
-	argc -= optind;
-	argv += optind;
-
-	if (argc < 1) {
-		printf("%s", key_help);
-		return;
-	}
+	helper_arg(1, 1, &argc, &argv, key_help);
 
 	str2ba(argv[0], &bdaddr);
 
@@ -2221,13 +2156,7 @@ static void cmd_clkoff(int dev_id, int argc, char **argv)
 			return;
 		}
 	}
-	argc -= optind;
-	argv += optind;
-
-	if (argc < 1) {
-		printf("%s", clkoff_help);
-		return;
-	}
+	helper_arg(1, 1, &argc, &argv, clkoff_help);
 
 	str2ba(argv[0], &bdaddr);
 
@@ -2297,8 +2226,7 @@ static void cmd_clock(int dev_id, int argc, char **argv)
 			return;
 		}
 	}
-	argc -= optind;
-	argv += optind;
+	helper_arg(0, 2, &argc, &argv, clock_help);
 
 	if (argc > 0)
 		str2ba(argv[0], &bdaddr);
@@ -2439,6 +2367,7 @@ static void cmd_lescan(int dev_id, int argc, char **argv)
 			return;
 		}
 	}
+	helper_arg(0, 0, &argc, &argv, lescan_help);
 
 	if (dev_id < 0)
 		dev_id = hci_get_route(NULL);
@@ -2503,14 +2432,7 @@ static void cmd_lecc(int dev_id, int argc, char **argv)
 			return;
 		}
 	}
-
-	argc -= optind;
-	argv += optind;
-
-	if (argc < 1) {
-		printf("%s", lecc_help);
-		return;
-	}
+	helper_arg(1, 1, &argc, &argv, lecc_help);
 
 	if (dev_id < 0)
 		dev_id = hci_get_route(NULL);
@@ -2571,14 +2493,7 @@ static void cmd_ledc(int dev_id, int argc, char **argv)
 			return;
 		}
 	}
-
-	argc -= optind;
-	argv += optind;
-
-	if (argc < 1) {
-		printf("%s", ledc_help);
-		return;
-	}
+	helper_arg(1, 2, &argc, &argv, ledc_help);
 
 	if (dev_id < 0)
 		dev_id = hci_get_route(NULL);
@@ -2699,10 +2614,18 @@ int main(int argc, char *argv[])
 	}
 
 	for (i = 0; command[i].cmd; i++) {
-		if (strncmp(command[i].cmd, argv[0], 3))
+		if (strncmp(command[i].cmd,
+				argv[0], strlen(command[i].cmd)))
 			continue;
+
 		command[i].func(dev_id, argc, argv);
 		break;
 	}
+
+	if (command[i].cmd == 0) {
+		fprintf(stderr, "Unknown command - \"%s\"\n", *argv);
+		exit(1);
+	}
+
 	return 0;
 }
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 1/3] Filter device name in hciconfig.
From: Michal Labedzki @ 2010-12-20 10:13 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Michal Labedzki
In-Reply-To: <1292840029-27770-1-git-send-email-michal.labedzki@tieto.com>

No anymore work someting like "hciconfig tty1", "hciconfig qfg1" or
"hciconfig hci1hg" as "hci1".
---
 lib/hci.c         |   12 +++++++++++-
 tools/hciconfig.c |   30 +++++++++++++++++++++++++++++-
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/lib/hci.c b/lib/hci.c
index 3304daa..64c7dad 100644
--- a/lib/hci.c
+++ b/lib/hci.c
@@ -54,6 +54,16 @@ typedef struct {
 	unsigned int val;
 } hci_map;
 
+static int is_number(const char *c)
+{
+	while(*c) {
+		if (! isdigit(*c))
+			return 0;
+		++c;
+	}
+	return 1;
+}
+
 static char *hci_bit2str(hci_map *m, unsigned int val)
 {
 	char *str = malloc(120);
@@ -889,7 +899,7 @@ int hci_devid(const char *str)
 	bdaddr_t ba;
 	int id = -1;
 
-	if (!strncmp(str, "hci", 3) && strlen(str) >= 4) {
+	if (!strncmp(str, "hci", 3) && strlen(str) >= 4 && is_number(str+3)) {
 		id = atoi(str + 3);
 		if (hci_devba(id, &ba) < 0)
 			return -1;
diff --git a/tools/hciconfig.c b/tools/hciconfig.c
index 3627b7c..e8b0c69 100644
--- a/tools/hciconfig.c
+++ b/tools/hciconfig.c
@@ -1721,6 +1721,33 @@ static void print_dev_info(int ctl, struct hci_dev_info *di)
 	printf("\n");
 }
 
+static int is_number(const char *c)
+{
+	while(*c) {
+		if (! isdigit(*c))
+			return 0;
+		++c;
+	}
+	return 1;
+}
+
+static int dev_name_filter(char *dev_name)
+{
+	int ret;
+
+	if ( (strlen(dev_name) >= 4) && (!strncmp(dev_name, "hci", 3)) &&
+			(is_number(dev_name+3)) )
+		ret = atoi(dev_name+3);
+	else if (is_number(dev_name))
+		ret = atoi(dev_name);
+	else {
+		fprintf(stderr, "No valid device name\n");
+		exit(1);
+	}
+
+	return ret;
+}
+
 static struct {
 	char *cmd;
 	void (*func)(int ctl, int hdev, char *opt);
@@ -1824,7 +1851,8 @@ int main(int argc, char *argv[])
 		exit(0);
 	}
 
-	di.dev_id = atoi(argv[0] + 3);
+	di.dev_id = dev_name_filter(argv[0]);
+
 	argc--; argv++;
 
 	if (ioctl(ctl, HCIGETDEVINFO, (void *) &di)) {
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH] Tools user interface improve
From: Michal Labedzki @ 2010-12-20 10:13 UTC (permalink / raw)
  To: linux-bluetooth


Hi,

Normally tools allow to put non-existent or mistake type arguments and
that do not have any impact on the program. But "hciconfig tty1" or
"hcitool name 'fooX' rm file.c" hide important error. Patches make tools more
hermetic, more information.

Regards

^ permalink raw reply

* Re: [PATCH 3/3] Fix memory leaks in btio/btio.c
From: Luiz Augusto von Dentz @ 2010-12-20  9:20 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <1292819346-27318-3-git-send-email-anderson.lizardo@openbossa.org>

Hi,

On Mon, Dec 20, 2010 at 6:29 AM, Anderson Lizardo
<anderson.lizardo@openbossa.org> wrote:
> The watches created by bt_io_* functions were not being removed from
> main context, therefore the "destroy" function was never being called.
> ---
>  btio/btio.c |   56 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
>  btio/btio.h |    2 ++
>  src/main.c  |    3 +++
>  3 files changed, 56 insertions(+), 5 deletions(-)
>
> diff --git a/btio/btio.c b/btio/btio.c
> index d8439e0..0cd046e 100644
> --- a/btio/btio.c
> +++ b/btio/btio.c
> @@ -65,12 +65,14 @@ struct connect {
>        BtIOConnect connect;
>        gpointer user_data;
>        GDestroyNotify destroy;
> +       guint watch_id;
>  };
>
>  struct accept {
>        BtIOConnect connect;
>        gpointer user_data;
>        GDestroyNotify destroy;
> +       guint watch_id;
>  };
>
>  struct server {
> @@ -78,8 +80,11 @@ struct server {
>        BtIOConfirm confirm;
>        gpointer user_data;
>        GDestroyNotify destroy;
> +       guint watch_id;
>  };
>
> +static GSList *servers = NULL, *accepts = NULL, *connects = NULL;
> +
>  static void server_remove(struct server *server)
>  {
>        if (server->destroy)
> @@ -215,8 +220,11 @@ static void server_add(GIOChannel *io, BtIOConnect connect,
>        server->destroy = destroy;
>
>        cond = G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
> -       g_io_add_watch_full(io, G_PRIORITY_DEFAULT, cond, server_cb, server,
> -                                       (GDestroyNotify) server_remove);
> +       server->watch_id = g_io_add_watch_full(io, G_PRIORITY_DEFAULT, cond,
> +                                               server_cb, server,
> +                                               (GDestroyNotify) server_remove);
> +
> +       servers = g_slist_append(servers, server);
>  }
>
>  static void connect_add(GIOChannel *io, BtIOConnect connect,
> @@ -231,8 +239,11 @@ static void connect_add(GIOChannel *io, BtIOConnect connect,
>        conn->destroy = destroy;
>
>        cond = G_IO_OUT | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
> -       g_io_add_watch_full(io, G_PRIORITY_DEFAULT, cond, connect_cb, conn,
> +       conn->watch_id = g_io_add_watch_full(io, G_PRIORITY_DEFAULT, cond,
> +                                       connect_cb, conn,
>                                        (GDestroyNotify) connect_remove);
> +
> +       connects = g_slist_append(connects, conn);
>  }
>
>  static void accept_add(GIOChannel *io, BtIOConnect connect, gpointer user_data,
> @@ -247,8 +258,11 @@ static void accept_add(GIOChannel *io, BtIOConnect connect, gpointer user_data,
>        accept->destroy = destroy;
>
>        cond = G_IO_OUT | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
> -       g_io_add_watch_full(io, G_PRIORITY_DEFAULT, cond, accept_cb, accept,
> -                                       (GDestroyNotify) accept_remove);
> +       accept->watch_id = g_io_add_watch_full(io, G_PRIORITY_DEFAULT, cond,
> +                                               accept_cb, accept,
> +                                               (GDestroyNotify) accept_remove);
> +
> +       accepts = g_slist_append(accepts, accept);
>  }
>
>  static int l2cap_bind(int sock, const bdaddr_t *src, uint16_t psm,
> @@ -1313,6 +1327,38 @@ GIOChannel *bt_io_listen(BtIOType type, BtIOConnect connect,
>        return io;
>  }
>
> +void bt_io_destroy(void)
> +{
> +       GSList *l;
> +
> +       for (l = servers; l != NULL; l = g_slist_next(l)) {
> +               struct server *server = l->data;
> +
> +               if (server->watch_id > 0)
> +                       g_source_remove(server->watch_id);
> +       }
> +       g_slist_free(servers);
> +       servers = NULL;
> +
> +       for (l = connects; l != NULL; l = g_slist_next(l)) {
> +               struct connect *conn = l->data;
> +
> +               if (conn->watch_id > 0)
> +                       g_source_remove(conn->watch_id);
> +       }
> +       g_slist_free(connects);
> +       connects = NULL;
> +
> +       for (l = connects; l != NULL; l = g_slist_next(l)) {
> +               struct accept *accept = l->data;
> +
> +               if (accept->watch_id > 0)
> +                       g_source_remove(accept->watch_id);
> +       }
> +       g_slist_free(accepts);
> +       accepts = NULL;
> +}
> +
>  GQuark bt_io_error_quark(void)
>  {
>        return g_quark_from_static_string("bt-io-error-quark");
> diff --git a/btio/btio.h b/btio/btio.h
> index 53e8eaa..87e722f 100644
> --- a/btio/btio.h
> +++ b/btio/btio.h
> @@ -95,4 +95,6 @@ GIOChannel *bt_io_listen(BtIOType type, BtIOConnect connect,
>                                GDestroyNotify destroy, GError **err,
>                                BtIOOption opt1, ...);
>
> +void bt_io_destroy(void);
> +
>  #endif
> diff --git a/src/main.c b/src/main.c
> index 1aaa181..1b7ff40 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -56,6 +56,7 @@
>  #include "dbus-common.h"
>  #include "agent.h"
>  #include "manager.h"
> +#include "btio.h"
>
>  #ifdef HAVE_CAPNG
>  #include <cap-ng.h>
> @@ -495,6 +496,8 @@ int main(int argc, char *argv[])
>
>        agent_exit();
>
> +       bt_io_destroy();
> +
>        g_main_loop_unref(event_loop);
>
>        if (config)
> --
> 1.7.0.4

While I see the reference problem I don't really see how this could
help since the application doesn't know about internal watches only
the reference returned, so if the application releases its references
btio won't release its own and in fact it can still call the
application callbacks.


-- 
Luiz Augusto von Dentz
Computer Engineer

^ permalink raw reply

* RE: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
From: Par-Gunnar HJALMDAHL @ 2010-12-20  9:15 UTC (permalink / raw)
  To: Vitaly Wool, Linus Walleij
  Cc: Pavan Savoy, Alan Cox, Arnd Bergmann, Samuel Ortiz,
	Marcel Holtmann, linux-kernel@vger.kernel.org,
	linux-bluetooth@vger.kernel.org, Lukasz Rymanowski,
	Par-Gunnar Hjalmdahl
In-Reply-To: <AANLkTi=J1AdOgxA_S9p9Ts65PL3Mi5NXXWMf0f-=0AwN@mail.gmail.com>

Hi Vitaly,

> -----Original Message-----
> From: Vitaly Wool [mailto:vitalywool@gmail.com]
> Sent: den 19 december 2010 23:58
> To: Linus Walleij
> Cc: Par-Gunnar HJALMDAHL; Pavan Savoy; Alan Cox; Arnd Bergmann; Samuel
> Ortiz; Marcel Holtmann; linux-kernel@vger.kernel.org; linux-
> bluetooth@vger.kernel.org; Lukasz Rymanowski; Par-Gunnar Hjalmdahl
> Subject: Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
>=20
> Hi Linus,
>=20
> On Sun, Dec 19, 2010 at 10:23 PM, Linus Walleij
> <linus.walleij@linaro.org> wrote:
> > I'm slightly confused by different comment threads on this patch set.
>=20
> let me first first repost the part from Arnd's email on the
> architecture of the "shared transport" type of thing:
>=20
> > I believe we already agree it should be something like
> >
> >   bt   ti-radio    st-e-radio    ti-gps   st-e-gps  broadcom-radio
> ...
> >   |         |          |            |          |          |         |
> >   +---------+----------+---------+--+----------+----------+---------+
> >                                  |
> >                         common-hci-module
> >                                  |
> >                      +-----------+-----------+
> >                      |        |       |      |
> >                    serial    spi     i2c    ...
>=20
> I think an explanation on how the patchset from Par maps to this
> architecture could be a good starting point for confusion minimization
> :)
>=20
> Thanks,
>    Vitaly

I would say our design would map like this:
common-hci-module: cg2900_core
serial, spi, i2c,... : cg2900_uart together with hci_ldisc (for other trans=
ports it would be different files)
bt, ti-radio, st-e-radio,...: cg2900_chip together with btcg2900 and other =
users per channel (cg2900_char_devices for users in User space)
So it is not a 1-to-1 mapping for the upper parts. I would draw it like thi=
s:

                               bt   st-e-radio  st-e-gps
                                |         |          |
                                +---------+----------+
                                          |
                   ti-xx                st-e cg2900...
                     |                    |
                     +---------+----------+
                               |
                       common-hci-module
                               |
                   +-----------+-----------+
                   |        |       |      |
                 serial    spi     i2c    ...

The reason for this difference I've gone through before. Basically there ar=
e so many special behaviors and needed handling that is individual for each=
 chip (like startup and shutdown and in the case of CG2900 flow control ove=
r FM and BT channels for audio commands). If you then look at the users I g=
uess it would be possible to have one BT user, but it would have to be modi=
fied to handle vendor specific commands (as btcg2900 does with BT_ENABLE co=
mmand). As Arnd has drawn for FM and GPS the users would be completely indi=
vidual since they don't have a standardized  interface.

/P-G

^ permalink raw reply

* Re: [PATCH 1/3] Fix memory leak in src/rfkill.c
From: Johan Hedberg @ 2010-12-20  8:16 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <1292819346-27318-1-git-send-email-anderson.lizardo@openbossa.org>

Hi Lizardo,

On Mon, Dec 20, 2010, Anderson Lizardo wrote:
> Watches must be removed with g_source_remove() otherwise they leak a
> reference count to the GIOChannel and internal memory allocations.
> 
> Also g_io_channel_set_close_on_unref() is used, so there is no need to
> call g_io_channel_shutdown() by hand.

You do realize that the watch gets freed and removed from the main
context when the callback function returns FALSE, right? The only case
for all of these patches where you'd have unfreed memory is when you
stop iterating the mainloop while still having the sockets open (or even
if you close them during the very last iteration), i.e. when bluetoothd
is exiting. Is that the problem you're trying to fix here, i.e. getting
rid of unnecessary "noise" in the valgrind leak report?

Johan

^ permalink raw reply

* [PATCH 3/3] Fix memory leaks in btio/btio.c
From: Anderson Lizardo @ 2010-12-20  4:29 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1292819346-27318-1-git-send-email-anderson.lizardo@openbossa.org>

The watches created by bt_io_* functions were not being removed from
main context, therefore the "destroy" function was never being called.
---
 btio/btio.c |   56 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
 btio/btio.h |    2 ++
 src/main.c  |    3 +++
 3 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/btio/btio.c b/btio/btio.c
index d8439e0..0cd046e 100644
--- a/btio/btio.c
+++ b/btio/btio.c
@@ -65,12 +65,14 @@ struct connect {
 	BtIOConnect connect;
 	gpointer user_data;
 	GDestroyNotify destroy;
+	guint watch_id;
 };
 
 struct accept {
 	BtIOConnect connect;
 	gpointer user_data;
 	GDestroyNotify destroy;
+	guint watch_id;
 };
 
 struct server {
@@ -78,8 +80,11 @@ struct server {
 	BtIOConfirm confirm;
 	gpointer user_data;
 	GDestroyNotify destroy;
+	guint watch_id;
 };
 
+static GSList *servers = NULL, *accepts = NULL, *connects = NULL;
+
 static void server_remove(struct server *server)
 {
 	if (server->destroy)
@@ -215,8 +220,11 @@ static void server_add(GIOChannel *io, BtIOConnect connect,
 	server->destroy = destroy;
 
 	cond = G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
-	g_io_add_watch_full(io, G_PRIORITY_DEFAULT, cond, server_cb, server,
-					(GDestroyNotify) server_remove);
+	server->watch_id = g_io_add_watch_full(io, G_PRIORITY_DEFAULT, cond,
+						server_cb, server,
+						(GDestroyNotify) server_remove);
+
+	servers = g_slist_append(servers, server);
 }
 
 static void connect_add(GIOChannel *io, BtIOConnect connect,
@@ -231,8 +239,11 @@ static void connect_add(GIOChannel *io, BtIOConnect connect,
 	conn->destroy = destroy;
 
 	cond = G_IO_OUT | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
-	g_io_add_watch_full(io, G_PRIORITY_DEFAULT, cond, connect_cb, conn,
+	conn->watch_id = g_io_add_watch_full(io, G_PRIORITY_DEFAULT, cond,
+					connect_cb, conn,
 					(GDestroyNotify) connect_remove);
+
+	connects = g_slist_append(connects, conn);
 }
 
 static void accept_add(GIOChannel *io, BtIOConnect connect, gpointer user_data,
@@ -247,8 +258,11 @@ static void accept_add(GIOChannel *io, BtIOConnect connect, gpointer user_data,
 	accept->destroy = destroy;
 
 	cond = G_IO_OUT | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
-	g_io_add_watch_full(io, G_PRIORITY_DEFAULT, cond, accept_cb, accept,
-					(GDestroyNotify) accept_remove);
+	accept->watch_id = g_io_add_watch_full(io, G_PRIORITY_DEFAULT, cond,
+						accept_cb, accept,
+						(GDestroyNotify) accept_remove);
+
+	accepts = g_slist_append(accepts, accept);
 }
 
 static int l2cap_bind(int sock, const bdaddr_t *src, uint16_t psm,
@@ -1313,6 +1327,38 @@ GIOChannel *bt_io_listen(BtIOType type, BtIOConnect connect,
 	return io;
 }
 
+void bt_io_destroy(void)
+{
+	GSList *l;
+
+	for (l = servers; l != NULL; l = g_slist_next(l)) {
+		struct server *server = l->data;
+
+		if (server->watch_id > 0)
+			g_source_remove(server->watch_id);
+	}
+	g_slist_free(servers);
+	servers = NULL;
+
+	for (l = connects; l != NULL; l = g_slist_next(l)) {
+		struct connect *conn = l->data;
+
+		if (conn->watch_id > 0)
+			g_source_remove(conn->watch_id);
+	}
+	g_slist_free(connects);
+	connects = NULL;
+
+	for (l = connects; l != NULL; l = g_slist_next(l)) {
+		struct accept *accept = l->data;
+
+		if (accept->watch_id > 0)
+			g_source_remove(accept->watch_id);
+	}
+	g_slist_free(accepts);
+	accepts = NULL;
+}
+
 GQuark bt_io_error_quark(void)
 {
 	return g_quark_from_static_string("bt-io-error-quark");
diff --git a/btio/btio.h b/btio/btio.h
index 53e8eaa..87e722f 100644
--- a/btio/btio.h
+++ b/btio/btio.h
@@ -95,4 +95,6 @@ GIOChannel *bt_io_listen(BtIOType type, BtIOConnect connect,
 				GDestroyNotify destroy, GError **err,
 				BtIOOption opt1, ...);
 
+void bt_io_destroy(void);
+
 #endif
diff --git a/src/main.c b/src/main.c
index 1aaa181..1b7ff40 100644
--- a/src/main.c
+++ b/src/main.c
@@ -56,6 +56,7 @@
 #include "dbus-common.h"
 #include "agent.h"
 #include "manager.h"
+#include "btio.h"
 
 #ifdef HAVE_CAPNG
 #include <cap-ng.h>
@@ -495,6 +496,8 @@ int main(int argc, char *argv[])
 
 	agent_exit();
 
+	bt_io_destroy();
+
 	g_main_loop_unref(event_loop);
 
 	if (config)
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 2/3] Fix memory leak in src/sdpd-server.c
From: Anderson Lizardo @ 2010-12-20  4:29 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1292819346-27318-1-git-send-email-anderson.lizardo@openbossa.org>

Watches must be removed with g_source_remove() otherwise they leak a
reference count to the GIOChannel and internal memory allocations.

Also do a little refactoring to avoid too many global variables.
---
 src/sdpd-server.c |   83 +++++++++++++++++++++++++++++-----------------------
 1 files changed, 46 insertions(+), 37 deletions(-)

diff --git a/src/sdpd-server.c b/src/sdpd-server.c
index efd6fd0..fbc4c96 100644
--- a/src/sdpd-server.c
+++ b/src/sdpd-server.c
@@ -48,9 +48,11 @@
 #include "log.h"
 #include "sdpd.h"
 
-static GIOChannel *l2cap_io = NULL, *unix_io = NULL;
-
-static int l2cap_sock, unix_sock;
+static struct sock_info {
+	int sk;
+	GIOChannel *io;
+	guint watch_id;
+} l2cap_sk, unix_sk;
 
 /*
  * SDP server initialization on startup includes creating the
@@ -71,8 +73,8 @@ static int init_server(uint16_t mtu, int master, int compat)
 	register_server_service();
 
 	/* Create L2CAP socket */
-	l2cap_sock = socket(PF_BLUETOOTH, SOCK_SEQPACKET, BTPROTO_L2CAP);
-	if (l2cap_sock < 0) {
+	l2cap_sk.sk = socket(PF_BLUETOOTH, SOCK_SEQPACKET, BTPROTO_L2CAP);
+	if (l2cap_sk.sk < 0) {
 		error("opening L2CAP socket: %s", strerror(errno));
 		return -1;
 	}
@@ -82,14 +84,16 @@ static int init_server(uint16_t mtu, int master, int compat)
 	bacpy(&l2addr.l2_bdaddr, BDADDR_ANY);
 	l2addr.l2_psm = htobs(SDP_PSM);
 
-	if (bind(l2cap_sock, (struct sockaddr *) &l2addr, sizeof(l2addr)) < 0) {
+	if (bind(l2cap_sk.sk, (struct sockaddr *) &l2addr,
+							sizeof(l2addr)) < 0) {
 		error("binding L2CAP socket: %s", strerror(errno));
 		return -1;
 	}
 
 	if (master) {
 		int opt = L2CAP_LM_MASTER;
-		if (setsockopt(l2cap_sock, SOL_L2CAP, L2CAP_LM, &opt, sizeof(opt)) < 0) {
+		if (setsockopt(l2cap_sk.sk, SOL_L2CAP, L2CAP_LM, &opt,
+							sizeof(opt)) < 0) {
 			error("setsockopt: %s", strerror(errno));
 			return -1;
 		}
@@ -99,7 +103,8 @@ static int init_server(uint16_t mtu, int master, int compat)
 		memset(&opts, 0, sizeof(opts));
 		optlen = sizeof(opts);
 
-		if (getsockopt(l2cap_sock, SOL_L2CAP, L2CAP_OPTIONS, &opts, &optlen) < 0) {
+		if (getsockopt(l2cap_sk.sk, SOL_L2CAP, L2CAP_OPTIONS, &opts,
+								&optlen) < 0) {
 			error("getsockopt: %s", strerror(errno));
 			return -1;
 		}
@@ -107,25 +112,24 @@ static int init_server(uint16_t mtu, int master, int compat)
 		opts.omtu = mtu;
 		opts.imtu = mtu;
 
-		if (setsockopt(l2cap_sock, SOL_L2CAP, L2CAP_OPTIONS, &opts, sizeof(opts)) < 0) {
+		if (setsockopt(l2cap_sk.sk, SOL_L2CAP, L2CAP_OPTIONS, &opts,
+							sizeof(opts)) < 0) {
 			error("setsockopt: %s", strerror(errno));
 			return -1;
 		}
 	}
 
-	if (listen(l2cap_sock, 5) < 0) {
+	if (listen(l2cap_sk.sk, 5) < 0) {
 		error("listen: %s", strerror(errno));
 		return -1;
 	}
 
-	if (!compat) {
-		unix_sock = -1;
+	if (!compat)
 		return 0;
-	}
 
 	/* Create local Unix socket */
-	unix_sock = socket(PF_UNIX, SOCK_STREAM, 0);
-	if (unix_sock < 0) {
+	unix_sk.sk = socket(PF_UNIX, SOCK_STREAM, 0);
+	if (unix_sk.sk < 0) {
 		error("opening UNIX socket: %s", strerror(errno));
 		return -1;
 	}
@@ -136,12 +140,13 @@ static int init_server(uint16_t mtu, int master, int compat)
 
 	unlink(unaddr.sun_path);
 
-	if (bind(unix_sock, (struct sockaddr *) &unaddr, sizeof(unaddr)) < 0) {
+	if (bind(unix_sk.sk, (struct sockaddr *) &unaddr,
+							sizeof(unaddr)) < 0) {
 		error("binding UNIX socket: %s", strerror(errno));
 		return -1;
 	}
 
-	if (listen(unix_sock, 5) < 0) {
+	if (listen(unix_sk.sk, 5) < 0) {
 		error("listen UNIX socket: %s", strerror(errno));
 		return -1;
 	}
@@ -195,21 +200,19 @@ static gboolean io_accept_event(GIOChannel *chan, GIOCondition cond, gpointer da
 	GIOChannel *io;
 	int nsk;
 
-	if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) {
-		g_io_channel_unref(chan);
+	if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL))
 		return FALSE;
-	}
 
-	if (data == &l2cap_sock) {
+	if (data == &l2cap_sk) {
 		struct sockaddr_l2 addr;
 		socklen_t len = sizeof(addr);
 
-		nsk = accept(l2cap_sock, (struct sockaddr *) &addr, &len);
-	} else if (data == &unix_sock) {
+		nsk = accept(l2cap_sk.sk, (struct sockaddr *) &addr, &len);
+	} else if (data == &unix_sk) {
 		struct sockaddr_un addr;
 		socklen_t len = sizeof(addr);
 
-		nsk = accept(unix_sock, (struct sockaddr *) &addr, &len);
+		nsk = accept(unix_sk.sk, (struct sockaddr *) &addr, &len);
 	} else
 		return FALSE;
 
@@ -256,18 +259,20 @@ int start_sdp_server(uint16_t mtu, const char *did, uint32_t flags)
 		}
 	}
 
-	l2cap_io = g_io_channel_unix_new(l2cap_sock);
-	g_io_channel_set_close_on_unref(l2cap_io, TRUE);
+	l2cap_sk.io = g_io_channel_unix_new(l2cap_sk.sk);
+	g_io_channel_set_close_on_unref(l2cap_sk.io, TRUE);
 
-	g_io_add_watch(l2cap_io, G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
-					io_accept_event, &l2cap_sock);
+	l2cap_sk.watch_id = g_io_add_watch(l2cap_sk.io, G_IO_IN | G_IO_ERR |
+						G_IO_HUP | G_IO_NVAL,
+						io_accept_event, &l2cap_sk);
 
-	if (compat && unix_sock > fileno(stderr)) {
-		unix_io = g_io_channel_unix_new(unix_sock);
-		g_io_channel_set_close_on_unref(unix_io, TRUE);
+	if (compat) {
+		unix_sk.io = g_io_channel_unix_new(unix_sk.sk);
+		g_io_channel_set_close_on_unref(unix_sk.io, TRUE);
 
-		g_io_add_watch(unix_io, G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
-					io_accept_event, &unix_sock);
+		unix_sk.watch_id = g_io_add_watch(unix_sk.io, G_IO_IN |
+						G_IO_ERR | G_IO_HUP | G_IO_NVAL,
+						io_accept_event, &unix_sk);
 	}
 
 	return 0;
@@ -279,9 +284,13 @@ void stop_sdp_server(void)
 
 	sdp_svcdb_reset();
 
-	if (unix_io)
-		g_io_channel_unref(unix_io);
+	if (l2cap_sk.watch_id > 0)
+		g_source_remove(l2cap_sk.watch_id);
+	g_io_channel_unref(l2cap_sk.io);
+	memset(&l2cap_sk, 0, sizeof(l2cap_sk));
 
-	if (l2cap_io)
-		g_io_channel_unref(l2cap_io);
+	if (unix_sk.watch_id > 0)
+		g_source_remove(unix_sk.watch_id);
+	g_io_channel_unref(unix_sk.io);
+	memset(&unix_sk, 0, sizeof(unix_sk));
 }
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 1/3] Fix memory leak in src/rfkill.c
From: Anderson Lizardo @ 2010-12-20  4:29 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Anderson Lizardo

Watches must be removed with g_source_remove() otherwise they leak a
reference count to the GIOChannel and internal memory allocations.

Also g_io_channel_set_close_on_unref() is used, so there is no need to
call g_io_channel_shutdown() by hand.
---
 src/rfkill.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/rfkill.c b/src/rfkill.c
index 75397b8..6e9e040 100644
--- a/src/rfkill.c
+++ b/src/rfkill.c
@@ -139,6 +139,7 @@ static gboolean rfkill_event(GIOChannel *chan,
 }
 
 static GIOChannel *channel = NULL;
+static guint watch_id = 0;
 
 void rfkill_init(void)
 {
@@ -156,8 +157,8 @@ void rfkill_init(void)
 	channel = g_io_channel_unix_new(fd);
 	g_io_channel_set_close_on_unref(channel, TRUE);
 
-	g_io_add_watch(channel, G_IO_IN | G_IO_NVAL | G_IO_HUP | G_IO_ERR,
-							rfkill_event, NULL);
+	watch_id = g_io_add_watch(channel, G_IO_IN | G_IO_NVAL | G_IO_HUP |
+						G_IO_ERR, rfkill_event, NULL);
 }
 
 void rfkill_exit(void)
@@ -165,7 +166,9 @@ void rfkill_exit(void)
 	if (!channel)
 		return;
 
-	g_io_channel_shutdown(channel, TRUE, NULL);
+	if (watch_id > 0)
+		g_source_remove(watch_id);
+
 	g_io_channel_unref(channel);
 
 	channel = NULL;
-- 
1.7.0.4


^ permalink raw reply related

* Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
From: Vitaly Wool @ 2010-12-19 22:57 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Par-Gunnar Hjalmdahl, Pavan Savoy, Alan Cox, Arnd Bergmann,
	Samuel Ortiz, Marcel Holtmann, linux-kernel, linux-bluetooth,
	Lukasz Rymanowski, Par-Gunnar Hjalmdahl
In-Reply-To: <AANLkTimCYea+G7Z+o-2BDCGHbRobM2n+VJLLcDO6g=9d@mail.gmail.com>

Hi Linus,

On Sun, Dec 19, 2010 at 10:23 PM, Linus Walleij
<linus.walleij@linaro.org> wrote:
> I'm slightly confused by different comment threads on this patch set.

let me first first repost the part from Arnd's email on the
architecture of the "shared transport" type of thing:

> I believe we already agree it should be something like
>
>   bt   ti-radio    st-e-radio    ti-gps   st-e-gps  broadcom-radio ...
>   |         |          |            |          |          |         |
>   +---------+----------+---------+--+----------+----------+---------+
>                                  |
>                         common-hci-module
>                                  |
>                      +-----------+-----------+
>                      |        |       |      |
>                    serial    spi     i2c    ...

I think an explanation on how the patchset from Par maps to this
architecture could be a good starting point for confusion minimization
:)

Thanks,
   Vitaly

^ permalink raw reply

* Re: [PATCH 00/11] mfd and bluetooth: Add CG2900 support
From: Linus Walleij @ 2010-12-19 21:23 UTC (permalink / raw)
  To: Par-Gunnar Hjalmdahl
  Cc: Pavan Savoy, Vitaly Wool, Alan Cox, Arnd Bergmann, Samuel Ortiz,
	Marcel Holtmann, linux-kernel, linux-bluetooth, Lukasz Rymanowski,
	Par-Gunnar Hjalmdahl
In-Reply-To: <1292584829-28279-1-git-send-email-par-gunnar.p.hjalmdahl@stericsson.com>

2010/12/17 Par-Gunnar Hjalmdahl <par-gunnar.p.hjalmdahl@stericsson.com>:

> This is the 3rd patch set for the ST-Ericsson CG2900 connectivity
> controller. The CG2900 is a combo controller supporting GPS, Bluetooth,
> and FM radio. It uses HCI H:4 protocol to combine different functionalities
> on a common transport, where first byte in the data indicates the current
> channel. Channels 1-4 are standardized in the Bluetooth Core specification
> while the other channels are vendor specific.

I'm slightly confused by different comment threads on this patch set.

I would certainly appreciate if the subsystem maintainers and reviewers
who shaped this patch set could add their Acked-by to the parts
they're happy with.

Alan, Marcel, Sam & Arnd especially. (Your work is MUCH
appreciated.)

I'm trying to get an idea of what patches are OK and what patches
are being disputed here, so as to whether there is some consensus
or not.

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH 1/4] Move get_eir_uuids() from src/adapter.c to src/event.c
From: Johan Hedberg @ 2010-12-19 12:27 UTC (permalink / raw)
  To: Bruna Moreira; +Cc: linux-bluetooth, Anderson Lizardo
In-Reply-To: <1292597844-17135-1-git-send-email-bruna.moreira@openbossa.org>

Hi,

On Fri, Dec 17, 2010, Bruna Moreira wrote:
> Moving get_eir_uuids() to src/event.c removes the need to pass the raw
> EIR data to higher layers. Now it is not necessary to pass the original
> GSList of service UUIDs, because the list is concatenated (with
> verification of duplicate entries) on adapter_update_device_from_info()
> (for LE) and adapter_update_found_devices() (for BR/EDR).
> ---
>  src/adapter.c |  145 +++++++++++---------------------------------------------
>  src/adapter.h |    7 +--
>  src/event.c   |  114 +++++++++++++++++++++++++++++++++++++++++++--
>  3 files changed, 142 insertions(+), 124 deletions(-)

All four patches have been pushed upstream. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH] Add enc_read_blob_req() as defined in BT Core Spec v4.0
From: Johan Hedberg @ 2010-12-19 12:13 UTC (permalink / raw)
  To: Brian Gix; +Cc: rshaffer, padovan, linux-bluetooth
In-Reply-To: <1292612933-31095-2-git-send-email-bgix@codeaurora.org>

Hi Brian,

On Fri, Dec 17, 2010, Brian Gix wrote:
> ---
>  attrib/att.c |   19 +++++++++++++++++++
>  attrib/att.h |    2 ++
>  2 files changed, 21 insertions(+), 0 deletions(-)

The patch has been pushed upstream. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH] Fix memory leak in adapter_service_ins_rem()
From: Johan Hedberg @ 2010-12-19  8:48 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <1292723784-10338-1-git-send-email-anderson.lizardo@openbossa.org>

Hi Lizardo,

On Sat, Dec 18, 2010, Anderson Lizardo wrote:
> ---
>  src/adapter.c |    9 ++++++---
>  1 files changed, 6 insertions(+), 3 deletions(-)

Thanks for catching this, however I went ahead and pushed a slightly
different (and imho simpler) fix. manager_find_adapter needs to iterate
through adapters while comparing addresses anyway so we might as well do
the address comparison within the for-loop in adapter_service_ins_rem().
That way there's no need to create an artificial list for the specific
adapter case.

Johan

^ permalink raw reply

* [PATCH] Fix memory leak in adapter_service_ins_rem()
From: Anderson Lizardo @ 2010-12-19  1:56 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Anderson Lizardo

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

diff --git a/src/adapter.c b/src/adapter.c
index 5118306..407522f 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -980,7 +980,7 @@ static void adapter_service_ins_rem(const bdaddr_t *bdaddr, void *rec,
 							gboolean insert)
 {
 	struct btd_adapter *adapter;
-	GSList *adapters;
+	GSList *l, *adapters;
 
 	adapters = NULL;
 
@@ -995,8 +995,8 @@ static void adapter_service_ins_rem(const bdaddr_t *bdaddr, void *rec,
 		/* Emit D-Bus msg to all adapters */
 		adapters = manager_get_adapters();
 
-	for (; adapters; adapters = adapters->next) {
-		adapter = adapters->data;
+	for (l = adapters; l; l = l->next) {
+		adapter = l->data;
 
 		if (insert == TRUE)
 			adapter->services = sdp_list_insert_sorted(
@@ -1008,6 +1008,9 @@ static void adapter_service_ins_rem(const bdaddr_t *bdaddr, void *rec,
 
 		adapter_emit_uuids_updated(adapter);
 	}
+
+	if (bacmp(bdaddr, BDADDR_ANY) != 0)
+		g_slist_free(adapters);
 }
 
 void adapter_service_insert(const bdaddr_t *bdaddr, void *rec)
-- 
1.7.0.4


^ permalink raw reply related

* [RFC 2/2] Fix gatt_read_char() to work with long attributes
From: Brian Gix @ 2010-12-17 22:48 UTC (permalink / raw)
  Cc: rshaffer, padovan, linux-bluetooth, Brian Gix
In-Reply-To: <1292626132-30029-1-git-send-email-bgix@codeaurora.org>

Fix gatt_read_char() so that it can correctly read an attribute longer
 	than (MTU-1) octets long. This is intended to be the first
 	use of the compound (multi-step) GATT Procedure mechanism
 	g_attrib_send_seq(), which will run the GATT procedure to completion
 	before executing any additional GATT Procedures. Functionaly, it
 	will check the result length of the original READ_REQ, and if
 	it is equal to the DEAFULT_MTU (23) octets long, it will make
 	a series of READ_BLOB_REQs until the entire remote attribute has
 	been read, after which it returns the result to the original
 	caller as if the data had been retrieved in a single ATT resp.

---
 attrib/gatt.c |  134 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 132 insertions(+), 2 deletions(-)

diff --git a/attrib/gatt.c b/attrib/gatt.c
index bca8b49..3a89b30 100644
--- a/attrib/gatt.c
+++ b/attrib/gatt.c
@@ -97,15 +97,145 @@ guint gatt_read_char_by_uuid(GAttrib *attrib, uint16_t start, uint16_t end,
 					pdu, plen, func, user_data, NULL);
 }
 
+struct read_long_data {
+	GAttrib *attrib;
+	GAttribResultFunc func;
+	gpointer user_data;
+	guint8 *buffer;
+	guint16 size;
+	guint16 handle;
+	guint id;
+	guint8 ref;
+};
+
+static void read_long_destroy(gpointer user_data)
+{
+	struct read_long_data *long_read = user_data;
+
+	if (--long_read->ref)
+		return;
+
+	if (long_read->buffer != NULL)
+		g_free(long_read->buffer);
+
+	g_free(long_read);
+}
+
+static void read_blob_helper(guint8 status, const guint8 *rpdu, guint16 rlen,
+							gpointer user_data)
+{
+	struct read_long_data *long_read = user_data;
+	uint8_t pdu[ATT_DEFAULT_MTU];
+	guint8 *tmp;
+	guint16 plen;
+	guint id;
+
+	if (status == ATT_ECODE_ATTR_NOT_LONG ||
+					status == ATT_ECODE_INVALID_OFFSET) {
+		status = 0;
+		goto done;
+	}
+
+	if (status != 0 || rlen == 1)
+		goto done;
+
+	tmp = g_try_realloc(long_read->buffer, long_read->size + rlen - 1);
+
+	if (tmp == NULL) {
+		status = ATT_ECODE_INSUFF_RESOURCES;
+		goto done;
+	}
+
+	memcpy(&tmp[long_read->size], &rpdu[1], rlen - 1);
+	long_read->buffer = tmp;
+	long_read->size += rlen - 1;
+
+	if (rlen < ATT_DEFAULT_MTU)
+		goto done;
+
+	plen = enc_read_blob_req(long_read->handle, long_read->size - 1,
+							pdu, sizeof(pdu));
+	id = g_attrib_send_seq(long_read->attrib, TRUE, long_read->id,
+				ATT_OP_READ_BLOB_REQ, pdu, plen,
+				read_blob_helper, long_read, read_long_destroy);
+
+	if (id != 0) {
+		long_read->ref++;
+		return;
+	}
+
+	status = ATT_ECODE_IO;
+
+done:
+	long_read->func(status, long_read->buffer, long_read->size,
+							long_read->user_data);
+}
+
+static void read_char_helper(guint8 status, const guint8 *rpdu,
+					guint16 rlen, gpointer user_data)
+{
+	struct read_long_data *long_read = user_data;
+	uint8_t pdu[ATT_DEFAULT_MTU];
+	guint16 plen;
+	guint id;
+
+	if (status != 0 || rlen < ATT_DEFAULT_MTU)
+		goto done;
+
+	long_read->buffer = g_malloc(rlen);
+
+	if (long_read->buffer == NULL)
+		goto done;
+
+	memcpy(long_read->buffer, rpdu, rlen);
+	long_read->size = rlen;
+
+	plen = enc_read_blob_req(long_read->handle, rlen - 1, pdu, sizeof(pdu));
+	id = g_attrib_send_seq(long_read->attrib, TRUE, long_read->id,
+			ATT_OP_READ_BLOB_REQ, pdu, plen, read_blob_helper,
+			long_read, read_long_destroy);
+
+	if (id != 0) {
+		long_read->ref++;
+		return;
+	}
+
+	status = ATT_ECODE_IO;
+
+done:
+	long_read->func(status, rpdu, rlen, long_read->user_data);
+}
+
 guint gatt_read_char(GAttrib *attrib, uint16_t handle, GAttribResultFunc func,
 							gpointer user_data)
 {
 	uint8_t pdu[ATT_DEFAULT_MTU];
 	guint16 plen;
+	guint id;
+	struct read_long_data *long_read;
+
+	long_read = g_try_new0(struct read_long_data, 1);
+
+	if (long_read == NULL)
+		return 0;
+
+	long_read->attrib = attrib;
+	long_read->func = func;
+	long_read->user_data = user_data;
+	long_read->handle = handle;
 
 	plen = enc_read_req(handle, pdu, sizeof(pdu));
-	return g_attrib_send(attrib, ATT_OP_READ_REQ, pdu, plen, func,
-							user_data, NULL);
+	id = g_attrib_send_seq(attrib, TRUE, 0, ATT_OP_READ_REQ, pdu, plen,
+				read_char_helper, long_read, read_long_destroy);
+
+	if (id == 0)
+		g_free(long_read);
+	else {
+		long_read->ref++;
+		long_read->id = id;
+	}
+
+	return id;
 }
 
 guint gatt_write_char(GAttrib *attrib, uint16_t handle, uint8_t *value,
-- 
1.7.2.2


^ permalink raw reply related

* [RFC 1/2] Add g_attrib_send_seq()
From: Brian Gix @ 2010-12-17 22:48 UTC (permalink / raw)
  Cc: rshaffer, padovan, linux-bluetooth, Brian Gix
In-Reply-To: <1292626132-30029-1-git-send-email-bgix@codeaurora.org>

Add g_attrib_send_seq() as an extension to g_attrib_send().
 	g_attrib_send_seq() functionally queues an entire
 	"GATT Procedure" as defined in the BT Core v4.0.
 	The intention is that the full procedure is run
 	to completion before the next GATT Procedure is
 	started. Subsequent ATT requests to a continuing
 	procedure are added to the head of the Attrib queue.

Fix g_attrib_send() to be the degenerative case of g_attrib_send_seq()
	This function now chains to g_attrib_send_seq() with arguments
	indicating that it is *not* a compound (multi-step) GATT
	procedure, but rather that the entire procedure is performed
	with a single ATT opcode request/response.

Fix received_data() to recognize that incoming response is (or isn't)
	part of a compound Procedure, so that it waits for additional
	requests before servicing the Attrib queue.
---
 attrib/gattrib.c |   44 ++++++++++++++++++++++++++++++++++++--------
 attrib/gattrib.h |    4 ++++
 2 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index eace01b..8ef5d92 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -58,6 +58,7 @@ struct command {
 	guint16 len;
 	guint8 expected;
 	gboolean sent;
+	gboolean compound;
 	GAttribResultFunc func;
 	gpointer user_data;
 	GDestroyNotify notify;
@@ -285,6 +286,7 @@ static gboolean received_data(GIOChannel *io, GIOCondition cond, gpointer data)
 	uint8_t buf[512], status;
 	gsize len;
 	GIOStatus iostat;
+	gboolean compound = FALSE;
 
 	if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) {
 		attrib->read_watch = 0;
@@ -319,6 +321,8 @@ static gboolean received_data(GIOChannel *io, GIOCondition cond, gpointer data)
 		return attrib->events != NULL;
 	}
 
+	compound = cmd->compound;
+
 	if (buf[0] == ATT_OP_ERROR) {
 		status = buf[4];
 		goto done;
@@ -332,7 +336,8 @@ static gboolean received_data(GIOChannel *io, GIOCondition cond, gpointer data)
 	status = 0;
 
 done:
-	if (attrib->queue && g_queue_is_empty(attrib->queue) == FALSE)
+	if (!compound && attrib->queue &&
+			g_queue_is_empty(attrib->queue) == FALSE)
 		wake_up_sender(attrib);
 
 	if (cmd) {
@@ -340,6 +345,11 @@ done:
 			cmd->func(status, buf, len, cmd->user_data);
 
 		command_destroy(cmd);
+
+		if (compound && attrib->queue &&
+			g_queue_is_empty(attrib->queue) == FALSE)
+			wake_up_sender(attrib);
+
 	}
 
 	return TRUE;
@@ -367,12 +377,16 @@ GAttrib *g_attrib_new(GIOChannel *io)
 	return g_attrib_ref(attrib);
 }
 
-guint g_attrib_send(GAttrib *attrib, guint8 opcode, const guint8 *pdu,
-				guint16 len, GAttribResultFunc func,
-				gpointer user_data, GDestroyNotify notify)
+guint g_attrib_send_seq(GAttrib *attrib, gboolean compound, guint id,
+				guint8 opcode, const guint8 *pdu, guint16 len,
+				GAttribResultFunc func, gpointer user_data,
+				GDestroyNotify notify)
 {
 	struct command *c;
 
+	if (attrib == NULL || attrib->queue == NULL)
+		return 0;
+
 	c = g_try_new0(struct command, 1);
 	if (c == NULL)
 		return 0;
@@ -385,16 +399,30 @@ guint g_attrib_send(GAttrib *attrib, guint8 opcode, const guint8 *pdu,
 	c->func = func;
 	c->user_data = user_data;
 	c->notify = notify;
-	c->id = ++attrib->next_cmd_id;
+	c->compound = compound;
 
-	g_queue_push_tail(attrib->queue, c);
+	if (id) {
+		c->id = id;
+		g_queue_push_head(attrib->queue, c);
+	} else {
+		c->id = ++attrib->next_cmd_id;
+		g_queue_push_tail(attrib->queue, c);
 
-	if (g_queue_get_length(attrib->queue) == 1)
-		wake_up_sender(attrib);
+		if (g_queue_get_length(attrib->queue) == 1)
+			wake_up_sender(attrib);
+	}
 
 	return c->id;
 }
 
+guint g_attrib_send(GAttrib *attrib, guint8 opcode, const guint8 *pdu,
+				guint16 len, GAttribResultFunc func,
+				gpointer user_data, GDestroyNotify notify)
+{
+	return g_attrib_send_seq(attrib, FALSE, 0, opcode,
+				pdu, len, func, user_data, notify);
+}
+
 static gint command_cmp_by_id(gconstpointer a, gconstpointer b)
 {
 	const struct command *cmd = a;
diff --git a/attrib/gattrib.h b/attrib/gattrib.h
index 0940289..ade21bc 100644
--- a/attrib/gattrib.h
+++ b/attrib/gattrib.h
@@ -53,6 +53,10 @@ gboolean g_attrib_set_destroy_function(GAttrib *attrib,
 guint g_attrib_send(GAttrib *attrib, guint8 opcode, const guint8 *pdu,
 				guint16 len, GAttribResultFunc func,
 				gpointer user_data, GDestroyNotify notify);
+guint g_attrib_send_seq(GAttrib *attrib, gboolean compound, guint id,
+				guint8 opcode, const guint8 *pdu,
+				guint16 len, GAttribResultFunc func,
+				gpointer user_data, GDestroyNotify notify);
 gboolean g_attrib_cancel(GAttrib *attrib, guint id);
 gboolean g_attrib_cancel_all(GAttrib *attrib);
 
-- 
1.7.2.2


^ permalink raw reply related

* [RFC 0/1] Implement Compound (Multi-step) GATT Procedure Support
From: Brian Gix @ 2010-12-17 22:48 UTC (permalink / raw)
  Cc: rshaffer, padovan, linux-bluetooth


The following two proposed patches implement a method for correctly
staging GATT procedures that require multiple ATT req/resp exchanges.

The first RFC / PATCH is gattrib.[ch], which allows the caller to specify
a gboolean indicating that the command being queued is Compound
(even if the procedure ends up being single/atomic) and a queue ID
number, which allows the caller to cancel the GATT procedure at any
stage of the transaction.

IF -
The ATT opcode being queued is specified as compound, the resulting response
will not cause the next item in the queue to be sent until *after* the
response has been forwarded to the caller via the response callback.

IF -
The ATT opcode being queued is *not* compound, the resulting response
will service the pkt queue immediately (legacy functionality).

IF -
The ID passed to g_attrib_send_seq is ZERO, the command pkt will
be added to the TAIL of the queue, and a new ID assigned to it.
(Legacy functionality)

IF -
The ID passed to g_attrib_send_seq is NON-ZERO, the command pkt
will be added to the HEAD of the queue, causing it to be the next
pkt sent to the remote device.  The NON-ZERO ID is also then able
to cancel the command.


The second RFC / PATCH is to gatt.c. It modifies the existing gatt_read_char()
function to recognize the need for a compound Read procedure, and implements
it as a READ_REQ + READ_BLOB_REQ + READ_BLOB_REQ ...<etc> as needed, using
the g_attrib_send_seq() logic from the first RFC / PATCH.

-- 
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

^ permalink raw reply

* Re: [PATCH 0/1] Add enc_read_blob_req() as defined in BT Core v4.0
From: Brian Gix @ 2010-12-17 21:01 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1292612933-31095-1-git-send-email-bgix@codeaurora.org>

Hi All

On Fri, 2010-12-17 at 11:08 -0800, Brian Gix wrote:
> Trivial addition of ATT API to encode READ_BLOB_REQ pkt.
> 

I apologize for messing up what must seem to be trivial rules for what
information to put where in the patches I am trying to get incorporated.

This particular patch is in fact a trivial addition to the ATT code,
which I believe probably is identical in all respects (including WS I
hope) to how anyone else here would have implemented it.

My follow on to this will be multi-step GATT procedures, I am going to
try as an RFC first, as the concept of GATT procedures that contain more
than one ATT Request/Response will be something that undoubtedly will
cause people to want to offer input on.

I also apologize for the sporadic nature of communication this week, as
my workstation had a catastrophic failure which is taking some time to
recover from.

> --
> Brian Gix
> bgix@codeaurora.org
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
> --
> 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


^ permalink raw reply

* [PATCH] Add enc_read_blob_req() as defined in BT Core Spec v4.0
From: Brian Gix @ 2010-12-17 19:08 UTC (permalink / raw)
  Cc: rshaffer, padovan, linux-bluetooth, Brian Gix
In-Reply-To: <1292612933-31095-1-git-send-email-bgix@codeaurora.org>

---
 attrib/att.c |   19 +++++++++++++++++++
 attrib/att.h |    2 ++
 2 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/attrib/att.c b/attrib/att.c
index 445b192..f8dbc02 100644
--- a/attrib/att.c
+++ b/attrib/att.c
@@ -542,6 +542,25 @@ uint16_t enc_read_req(uint16_t handle, uint8_t *pdu, int len)
 	return min_len;
 }
 
+uint16_t enc_read_blob_req(uint16_t handle, uint16_t offset, uint8_t *pdu,
+									int len)
+{
+	const uint16_t min_len = sizeof(pdu[0]) + sizeof(handle) +
+							sizeof(offset);
+
+	if (pdu == NULL)
+		return 0;
+
+	if (len < min_len)
+		return 0;
+
+	pdu[0] = ATT_OP_READ_BLOB_REQ;
+	att_put_u16(handle, &pdu[1]);
+	att_put_u16(offset, &pdu[3]);
+
+	return min_len;
+}
+
 uint16_t dec_read_req(const uint8_t *pdu, int len, uint16_t *handle)
 {
 	const uint16_t min_len = sizeof(pdu[0]) + sizeof(*handle);
diff --git a/attrib/att.h b/attrib/att.h
index 2c8c724..0b8612e 100644
--- a/attrib/att.h
+++ b/attrib/att.h
@@ -202,6 +202,8 @@ uint16_t enc_write_req(uint16_t handle, const uint8_t *value, int vlen,
 uint16_t dec_write_req(const uint8_t *pdu, int len, uint16_t *handle,
 						uint8_t *value, int *vlen);
 uint16_t enc_read_req(uint16_t handle, uint8_t *pdu, int len);
+uint16_t enc_read_blob_req(uint16_t handle, uint16_t offset, uint8_t *pdu,
+									int len);
 uint16_t dec_read_req(const uint8_t *pdu, int len, uint16_t *handle);
 uint16_t enc_read_resp(uint8_t *value, int vlen, uint8_t *pdu, int len);
 uint16_t dec_read_resp(const uint8_t *pdu, int len, uint8_t *value, int *vlen);
-- 
1.7.2.2


^ permalink raw reply related

* [PATCH 0/1] Add enc_read_blob_req() as defined in BT Core v4.0
From: Brian Gix @ 2010-12-17 19:08 UTC (permalink / raw)
  Cc: rshaffer, padovan, linux-bluetooth


Trivial addition of ATT API to encode READ_BLOB_REQ pkt.

--
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

^ permalink raw reply

* [RFC][PATCH] Re: EDR support
From: Manuel Naranjo @ 2010-12-17 16:41 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: BlueZ
In-Reply-To: <1292585241.2658.0.camel@aeonflux>

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

Hi Marcel,
> Hi Manuel,
>
>> I was checking the latest BlueZ source code and the kernel source code,
>> and I noticed that by default EDR is disabled, when a dongle is detected
>> net/bluetooth/hci_core.c initializes the packet types with DM1, DH1 and
>> HV1, and then net/bluetooth/hci_event.c does it for DM3-5 DH3-5 and
>> HV3-5, but it never initializes the 2DHx or 3DHx. Was it made on purpose
>> or is it a bug in the code?
> you have read the specification and realized that EDR uses inverse
> values?
I can't find that, but still I could find that according to the 2.1 
specs + EDR when you want to enable EDR is up to the master or slave to 
ask for the package type switch, that means as far as I understand ask 
the other side to enable 2-DH1,2-DH3,2-DH5,3-DH1,3-DH3,3-DH5.

I get from this that from the BlueZ point of view is either the kernel 
that should do that, or the BlueZ daemon or just rely that to the user 
application.

But I think there are a few of non covered issues here, first when you 
attach an EDR dongle which has the EDR (this means is capable of doing 
2mbps and 3mbps acl connections the kernel side doesn't show this to the 
userland (hciconfig), I made a patch that does exactly that, during the 
device recognition of the kernel just read the features provided and or 
that to the pkt_type flag. This helps a lot, now my dongle shows it can 
do EDR:
EDR compatible:
         Features: 0xff 0xff 0x8f 0xfe 0x9b 0xff 0x59 0x83
         Packet type: DM1 DM3 DM5 DH1 DH3 DH5 HV1 HV2 HV3 2-DH1 2-DH3 
2-DH5 3-DH1 3-DH3 3-DH5

EDR not compatible:
         Features: 0xff 0xff 0x8f 0xf8 0x1b 0xf8 0x00 0x80
         Packet type: DM1 DM3 DM5 DH1 DH3 DH5 HV1 HV2 HV3

If you think the patch is sane to be included then I format it properly 
so far I made it against compat-wireless-2.6.36-4 but it wouldn't be an 
issue to make it against the current kernel source.

Next thing was playing with hcitool cpt and that did work, I get speed 
improvements, from ~ 60KBs I get up to 100KBs to the same device, just 
by forcing it to only do 3-DH5 (which I know is not a good idea, because 
it's best for both radios to adapt, based on the environment).

I think it's a good idea to rely EDR "enabling" to the user space, 
otherwise connection complexity on the kernel well be way harder to 
keep. I think the best would be to add some stuff to the DBUS so first 
let the user know which packet types are available, second to allow it 
to change on a new connection.

Please let me know what you think about it,

Manuel

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

diff -uprN compat-wireless-2.6.36-4/include/net/bluetooth/hci.h compat-wireless-2.6.36-4.mine//include/net/bluetooth/hci.h
--- compat-wireless-2.6.36-4/include/net/bluetooth/hci.h	2010-11-08 22:00:51.000000000 -0300
+++ compat-wireless-2.6.36-4.mine//include/net/bluetooth/hci.h	2010-12-11 19:13:14.000000000 -0300
@@ -1,6 +1,7 @@
 /* 
    BlueZ - Bluetooth protocol stack for Linux
    Copyright (C) 2000-2001 Qualcomm Incorporated
+   Copyright (C) 2010 Naranjo Manuel Francisco <manuel@aircable.net>
 
    Written 2000,2001 by Maxim Krasnyansky <maxk@qualcomm.com>
 
@@ -120,17 +121,31 @@ enum {
 #define HCI_VENDOR_PKT		0xff
 
 /* HCI packet types */
+#define HCI_2DH1	0x0002
+#define HCI_3DH1	0x0004
 #define HCI_DM1		0x0008
-#define HCI_DM3		0x0400
-#define HCI_DM5		0x4000
 #define HCI_DH1		0x0010
+#define HCI_2DH3	0x0100
+#define HCI_3DH3	0x0200
+#define HCI_DM3		0x0400
 #define HCI_DH3		0x0800
+#define HCI_2DH5	0x1000
+#define HCI_3DH5	0x2000
+#define HCI_DM5		0x4000
 #define HCI_DH5		0x8000
 
 #define HCI_HV1		0x0020
 #define HCI_HV2		0x0040
 #define HCI_HV3		0x0080
 
+#define HCI_EV3		0x0008
+#define HCI_EV4		0x0010
+#define HCI_EV5		0x0020
+#define HCI_2EV3	0x0040
+#define HCI_3EV3	0x0080
+#define HCI_2EV5	0x0100
+#define HCI_3EV5	0x0200
+
 #define SCO_PTYPE_MASK	(HCI_HV1 | HCI_HV2 | HCI_HV3)
 #define ACL_PTYPE_MASK	(~SCO_PTYPE_MASK)
 
@@ -183,6 +198,12 @@ enum {
 #define LMP_PSCHEME	0x02
 #define LMP_PCONTROL	0x04
 
+#define LMP_EDR_ACL_2M	0x02
+#define LMP_EDR_ACL_3M	0x04
+#define LMP_ENH_ISCAN	0x08
+#define LMP_ILACE_ISCAN	0x10
+#define LMP_ILACE_PSCAN	0x20
+#define LMP_RSSI_INQ	0x40
 #define LMP_ESCO	0x80
 
 #define LMP_EV4		0x01
diff -uprN compat-wireless-2.6.36-4/net/bluetooth/hci_event.c compat-wireless-2.6.36-4.mine//net/bluetooth/hci_event.c
--- compat-wireless-2.6.36-4/net/bluetooth/hci_event.c	2010-11-08 22:00:51.000000000 -0300
+++ compat-wireless-2.6.36-4.mine//net/bluetooth/hci_event.c	2010-12-11 19:13:46.000000000 -0300
@@ -1,6 +1,7 @@
 /*
    BlueZ - Bluetooth protocol stack for Linux
    Copyright (c) 2000-2001, 2010, Code Aurora Forum. All rights reserved.
+   Copyright (c) 2010 Naranjo Manuel Francisco <manuel@aircable.net>
 
    Written 2000,2001 by Maxim Krasnyansky <maxk@qualcomm.com>
 
@@ -478,6 +479,26 @@ static void hci_cc_read_local_features(s
 	if (hdev->features[3] & LMP_ESCO)
 		hdev->esco_type |= (ESCO_EV3);
 
+	if (hdev->features[3] & LMP_EDR_ACL_2M){
+		hdev->pkt_type |= (HCI_2DH1);
+
+		if (hdev->features[0] & LMP_3SLOT)
+		    hdev->pkt_type |= (HCI_2DH3);
+
+		if (hdev->features[0] & LMP_5SLOT)
+		    hdev->pkt_type |= (HCI_2DH5);
+	}
+
+	if (hdev->features[3] & LMP_EDR_ACL_3M){
+		hdev->pkt_type |= (HCI_3DH1);
+
+		if (hdev->features[0] & LMP_3SLOT)
+		    hdev->pkt_type |= (HCI_3DH3);
+
+		if (hdev->features[0] & LMP_5SLOT)
+		    hdev->pkt_type |= (HCI_3DH5);
+	}
+
 	if (hdev->features[4] & LMP_EV4)
 		hdev->esco_type |= (ESCO_EV4);
 

^ permalink raw reply

* [PATCH 4/4] Move local name parsing to parse_eir_data()
From: Bruna Moreira @ 2010-12-17 14:57 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1292597844-17135-1-git-send-email-bruna.moreira@openbossa.org>

From: Anderson Lizardo <anderson.lizardo@openbossa.org>

---
 src/adapter.c     |   12 +++++-------
 src/adapter.h     |    3 ++-
 src/event.c       |   34 ++++++++++++++++++++++------------
 src/glib-helper.c |   22 ----------------------
 src/glib-helper.h |    1 -
 5 files changed, 29 insertions(+), 43 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index c74019d..b826d4a 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2805,7 +2805,8 @@ static void remove_same_uuid(gpointer data, gpointer user_data)
 
 void adapter_update_device_from_info(struct btd_adapter *adapter,
 						le_advertising_info *info,
-						GSList *services, uint8_t flags)
+						char *name, GSList *services,
+						uint8_t flags)
 {
 	struct remote_dev_info *dev;
 	bdaddr_t bdaddr;
@@ -2833,12 +2834,9 @@ void adapter_update_device_from_info(struct btd_adapter *adapter,
 
 	dev->flags = flags;
 
-	if (info->length) {
-		char *tmp_name = bt_extract_eir_name(info->data, NULL);
-		if (tmp_name) {
-			g_free(dev->name);
-			dev->name = tmp_name;
-		}
+	if (name) {
+		g_free(dev->name);
+		dev->name = name;
 	}
 
 	/* FIXME: check if other information was changed before emitting the
diff --git a/src/adapter.h b/src/adapter.h
index d5dceb9..52417b6 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -125,7 +125,8 @@ struct remote_dev_info *adapter_search_found_devices(struct btd_adapter *adapter
 						struct remote_dev_info *match);
 void adapter_update_device_from_info(struct btd_adapter *adapter,
 						le_advertising_info *info,
-						GSList *services, uint8_t flags);
+						char *name, GSList *services,
+						uint8_t flags);
 void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
 				int8_t rssi, uint32_t class, const char *name,
 				const char *alias, gboolean legacy,
diff --git a/src/event.c b/src/event.c
index 4672106..e138312 100644
--- a/src/event.c
+++ b/src/event.c
@@ -61,6 +61,8 @@
 struct eir_data {
 	GSList *services;
 	uint8_t flags;
+	char *name;
+	gboolean name_complete;
 };
 
 static gboolean get_adapter_and_device(bdaddr_t *src, bdaddr_t *dst,
@@ -350,6 +352,16 @@ static int parse_eir_data(struct eir_data *eir, uint8_t *eir_data,
 		case EIR_FLAGS:
 			eir->flags = eir_data[2];
 			break;
+		case EIR_NAME_SHORT:
+		case EIR_NAME_COMPLETE:
+			if (g_utf8_validate((char *) &eir_data[2],
+							field_len - 1, NULL))
+				eir->name = g_strndup((char *) &eir_data[2],
+								field_len - 1);
+			else
+				eir->name = g_strdup("");
+			eir->name_complete = eir_data[1] == EIR_NAME_COMPLETE;
+			break;
 		}
 
 		len += field_len + 1;
@@ -426,8 +438,8 @@ void btd_event_advertising_report(bdaddr_t *local, le_advertising_info *info)
 		error("Error parsing advertising data: %s (%d)",
 							strerror(-err), -err);
 
-	adapter_update_device_from_info(adapter, info, eir_data.services,
-								eir_data.flags);
+	adapter_update_device_from_info(adapter, info, eir_data.name,
+					eir_data.services, eir_data.flags);
 }
 
 void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
@@ -436,9 +448,8 @@ void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
 	char filename[PATH_MAX + 1];
 	struct btd_adapter *adapter;
 	struct btd_device *device;
-	char local_addr[18], peer_addr[18], *alias, *name, *tmp_name;
+	char local_addr[18], peer_addr[18], *alias, *name;
 	struct remote_dev_info *dev, match;
-	uint8_t name_type = 0x00;
 	name_status_t name_status;
 	struct eir_data eir_data;
 	int state, err;
@@ -513,25 +524,24 @@ void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
 	} else
 		legacy = TRUE;
 
-	tmp_name = bt_extract_eir_name(data, &name_type);
-	if (tmp_name) {
-		if (name_type == 0x09) {
-			write_device_name(local, peer, tmp_name);
+	if (eir_data.name) {
+		if (eir_data.name_complete) {
+			write_device_name(local, peer, eir_data.name);
 			name_status = NAME_NOT_REQUIRED;
 
 			if (name)
 				g_free(name);
 
-			name = tmp_name;
+			name = eir_data.name;
 		} else {
 			if (name)
-				free(tmp_name);
+				free(eir_data.name);
 			else
-				name = tmp_name;
+				name = eir_data.name;
 		}
 	}
 
-	if (name && name_type != 0x08)
+	if (name && eir_data.name_complete)
 		name_status = NAME_SENT;
 
 	/* add in the list to track name sent/pending */
diff --git a/src/glib-helper.c b/src/glib-helper.c
index 6505249..c440e60 100644
--- a/src/glib-helper.c
+++ b/src/glib-helper.c
@@ -780,25 +780,3 @@ GSList *bt_string2list(const gchar *str)
 
 	return l;
 }
-
-char *bt_extract_eir_name(uint8_t *data, uint8_t *type)
-{
-	if (!data || !type)
-		return NULL;
-
-	if (data[0] == 0)
-		return NULL;
-
-	if (type)
-		*type = data[1];
-
-	switch (data[1]) {
-	case EIR_NAME_SHORT:
-	case EIR_NAME_COMPLETE:
-		if (!g_utf8_validate((char *) (data + 2), data[0] - 1, NULL))
-			return g_strdup("");
-		return g_strndup((char *) (data + 2), data[0] - 1);
-	}
-
-	return NULL;
-}
diff --git a/src/glib-helper.h b/src/glib-helper.h
index 5bb20a6..10718bb 100644
--- a/src/glib-helper.h
+++ b/src/glib-helper.h
@@ -42,4 +42,3 @@ char *bt_name2string(const char *string);
 int bt_string2uuid(uuid_t *uuid, const char *string);
 gchar *bt_list2string(GSList *list);
 GSList *bt_string2list(const gchar *str);
-char *bt_extract_eir_name(uint8_t *data, uint8_t *type);
-- 
1.7.0.4


^ permalink raw reply related

* [PATCH 3/4] Move AD flags parsing to parse_eir_data()
From: Bruna Moreira @ 2010-12-17 14:57 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1292597844-17135-1-git-send-email-bruna.moreira@openbossa.org>

From: Anderson Lizardo <anderson.lizardo@openbossa.org>

---
 src/adapter.c |   22 ++++------------------
 src/adapter.h |    3 ++-
 src/event.c   |    7 ++++++-
 3 files changed, 12 insertions(+), 20 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 34bf24a..c74019d 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2786,21 +2786,6 @@ static struct remote_dev_info *get_found_dev(struct btd_adapter *adapter,
 	return dev;
 }
 
-static gboolean extract_eir_flags(uint8_t *flags, uint8_t *eir_data)
-{
-	if (eir_data[0] == 0)
-		return FALSE;
-
-	if (eir_data[1] != EIR_FLAGS)
-		return FALSE;
-
-	/* For now, only one octet is used for flags */
-	if (flags)
-		*flags = eir_data[2];
-
-	return TRUE;
-}
-
 static void remove_same_uuid(gpointer data, gpointer user_data)
 {
 	struct remote_dev_info *dev = user_data;
@@ -2819,7 +2804,8 @@ static void remove_same_uuid(gpointer data, gpointer user_data)
 }
 
 void adapter_update_device_from_info(struct btd_adapter *adapter,
-				le_advertising_info *info, GSList *services)
+						le_advertising_info *info,
+						GSList *services, uint8_t flags)
 {
 	struct remote_dev_info *dev;
 	bdaddr_t bdaddr;
@@ -2845,14 +2831,14 @@ void adapter_update_device_from_info(struct btd_adapter *adapter,
 	g_slist_foreach(services, remove_same_uuid, dev);
 	dev->services = g_slist_concat(dev->services, services);
 
+	dev->flags = flags;
+
 	if (info->length) {
 		char *tmp_name = bt_extract_eir_name(info->data, NULL);
 		if (tmp_name) {
 			g_free(dev->name);
 			dev->name = tmp_name;
 		}
-
-		extract_eir_flags(info->data, &dev->flags);
 	}
 
 	/* FIXME: check if other information was changed before emitting the
diff --git a/src/adapter.h b/src/adapter.h
index efcf5b8..d5dceb9 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -124,7 +124,8 @@ gboolean adapter_is_ready(struct btd_adapter *adapter);
 struct remote_dev_info *adapter_search_found_devices(struct btd_adapter *adapter,
 						struct remote_dev_info *match);
 void adapter_update_device_from_info(struct btd_adapter *adapter,
-				le_advertising_info *info, GSList *services);
+						le_advertising_info *info,
+						GSList *services, uint8_t flags);
 void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
 				int8_t rssi, uint32_t class, const char *name,
 				const char *alias, gboolean legacy,
diff --git a/src/event.c b/src/event.c
index 3352717..4672106 100644
--- a/src/event.c
+++ b/src/event.c
@@ -60,6 +60,7 @@
 
 struct eir_data {
 	GSList *services;
+	uint8_t flags;
 };
 
 static gboolean get_adapter_and_device(bdaddr_t *src, bdaddr_t *dst,
@@ -346,6 +347,9 @@ static int parse_eir_data(struct eir_data *eir, uint8_t *eir_data,
 			uuid128_count = field_len / 16;
 			uuid128 = &eir_data[2];
 			break;
+		case EIR_FLAGS:
+			eir->flags = eir_data[2];
+			break;
 		}
 
 		len += field_len + 1;
@@ -422,7 +426,8 @@ void btd_event_advertising_report(bdaddr_t *local, le_advertising_info *info)
 		error("Error parsing advertising data: %s (%d)",
 							strerror(-err), -err);
 
-	adapter_update_device_from_info(adapter, info, eir_data.services);
+	adapter_update_device_from_info(adapter, info, eir_data.services,
+								eir_data.flags);
 }
 
 void btd_event_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
-- 
1.7.0.4


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox