* Re: [PATCH v2] Fix memory leak of unused EIR name
From: Johan Hedberg @ 2010-12-20 15:36 UTC (permalink / raw)
To: anderson.lizardo; +Cc: linux-bluetooth
In-Reply-To: <4d0f7725.81a5e60a.143c.2315@mx.google.com>
Hi Lizardo,
On Mon, Dec 20, 2010, anderson.lizardo@openbossa.org wrote:
> There is a code path in btd_event_device_found() where the EIR name is
> not used. This requires freeing eir_data.name to avoid a leak.
> ---
> src/event.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
Thanks. Pushed upstream.
Johan
^ permalink raw reply
* [PATCH v2] Fix memory leak of unused EIR name
From: anderson.lizardo @ 2010-12-20 15:32 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1292852295-11104-1-git-send-email-anderson.lizardo@openbossa.org>
From: Anderson Lizardo <anderson.lizardo@openbossa.org>
There is a code path in btd_event_device_found() where the EIR name is
not used. This requires freeing eir_data.name to avoid a leak.
---
src/event.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/event.c b/src/event.c
index 3603643..22f4f72 100644
--- a/src/event.c
+++ b/src/event.c
@@ -527,6 +527,7 @@ void btd_event_device_found(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
/* if found: don't send the name again */
dev = adapter_search_found_devices(adapter, &match);
if (dev) {
+ g_free(eir_data.name);
adapter_update_found_devices(adapter, peer, rssi, class,
NULL, NULL, dev->legacy,
eir_data.services,
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH 2/5] Change CreatePairedDevice to support LE devices
From: Claudio Takahasi @ 2010-12-20 14:03 UTC (permalink / raw)
To: Brian Gix; +Cc: linux-bluetooth, Sheldon Demario
In-Reply-To: <1292539367.6740.13.camel@sealablnx02.qualcomm.com>
Hi Brian,
On Thu, Dec 16, 2010 at 7:42 PM, Brian Gix <bgix@codeaurora.org> wrote:
> Hi Claudio,
>
> On Wed, 2010-12-15 at 16:54 -0300, Claudio Takahasi wrote:
>> From: Sheldon Demario <sheldon.demario@openbossa.org>
>>
>> CreatePairedDevice implements now the same behaviour of CreateDevice,
>> triggering Discover All Primary Services when needed. SMP negotiation
>> starts when the link is established. LE capable kernel is required to
>> test this method properly.
>
> What is your plan for handling single mode LE (remote) devices which
> have privacy enabled (random addresses). This impacts connection
> establishment, and SM pairing, as the remote device's addr type must
> be known at that time, and handled differently.
At the moment, we don't have a clear picture how we will address
random address. For this first phase I was considering to support
public address only.
The kernel doesn't have persistent storage of the key, we will
implement it using the new kernel management interface.
We discussed the possibility of translate/resolve the address in the
kernel, but we didn't reach a conclusion yet.
Regards,
Claudio.
^ permalink raw reply
* [PATCH] Fix memory leak of unused EIR name
From: Anderson Lizardo @ 2010-12-20 13:38 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
There is a code path in btd_event_device_found() where the EIR name is
not used. This requires freeing eir_data.name to avoid a leak.
Additionally, an incorrect use of free() is replaced with g_free().
---
src/event.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/event.c b/src/event.c
index 3603643..0c11da7 100644
--- a/src/event.c
+++ b/src/event.c
@@ -527,6 +527,7 @@ void btd_event_device_found(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
/* if found: don't send the name again */
dev = adapter_search_found_devices(adapter, &match);
if (dev) {
+ g_free(eir_data.name);
adapter_update_found_devices(adapter, peer, rssi, class,
NULL, NULL, dev->legacy,
eir_data.services,
@@ -570,7 +571,7 @@ void btd_event_device_found(bdaddr_t *local, bdaddr_t *peer, uint32_t class,
name = eir_data.name;
} else {
if (name)
- free(eir_data.name);
+ g_free(eir_data.name);
else
name = eir_data.name;
}
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH 3/3] Fix memory leaks in btio/btio.c
From: Anderson Lizardo @ 2010-12-20 11:57 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <AANLkTi=9iFhHSK2p21eCDK8mT9Y-BPagCTd4XhTyGCer@mail.gmail.com>
On Mon, Dec 20, 2010 at 7:36 AM, Anderson Lizardo
<anderson.lizardo@openbossa.org> wrote:
> Hi Luiz,
>
> On Mon, Dec 20, 2010 at 5:20 AM, Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
>> 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.
After Johan's explanation on IRC about btio policy regarding keeping
state, I think I understand your comment now and I believe the patch
can be just ignored.
Regards,
--
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [PATCH 3/3] Fix memory leaks in btio/btio.c
From: Anderson Lizardo @ 2010-12-20 11:36 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <AANLkTimNkaxqTShFMr4-5Dk+i3iFKDz_b6wHNKb-0fQX@mail.gmail.com>
Hi Luiz,
On Mon, Dec 20, 2010 at 5:20 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> 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.
Sorry, but I don't understand the issue you describe here. The watch
created by e.g. bt_io_listen() needs to be destroyed at some point,
otherwise any destroy callbacks registered by the btio users will
never be called, and the reference added by the watch itself is not
decremented. You can see this by running valgrind with
--leak-check=yes with/without these patches.
Doing this during the daemon shutdown seems a good place, as it will
do other memory deallocations anyway.
Regards,
--
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil
^ permalink raw reply
* [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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox