BLE devices have two different address types: public or
random. So we need to provide this address type on keys in
the "primary" file to avoid duplicate BLE address types
around the BlueZ.
This series changes the storage to store the address type
together with the device address.
Hemant Gupta has been working on this on "Store LE device
address type with primary list" but his patch hasn't been
accepted and he didn't send a new reviewed version of that
patch.
I have patches for the other storage files as well, I'll send
them when this series is integrated upstream.
Claudio Takahasi (1):
core: Fix creating device from "primary" file
Paulo Alcantara (1):
storage: Store BLE address type in "primary" file
src/adapter.c | 10 +++++++---
src/device.c | 2 +-
src/storage.c | 30 ++++++++++++++++++++++++++----
src/storage.h | 5 +++--
4 files changed, 37 insertions(+), 10 deletions(-)
--
1.7.7.6
Hi Paulo,
On Tue, May 22, 2012, Paulo Alcantara wrote:
> BLE devices have two different address types: public or
> random. So we need to provide this address type on keys
> in the "primary" file to avoid duplicate BLE address types
> around the BlueZ.
>
> This series changes the storage to store the address type
> together with the device address.
>
> Hemant Gupta has been working on this on "Store LE device address
> type with primary list" but his patch hasn't been accepted and
> he didn't send a new reviewed version of that patch.
>
> I have patches for the other storage files as well, I'll
> send them soon. There is a potential problem in
> device_address_cmp() function which checks the address only.
> I am investigating how to fix it to compare address and type
> without breaking the current code.
>
> v3's changes:
> - (1/3): Remove unnecessary comment about the new storage
> format and code that leads with the old storage
> format.
>
> - (2/3): Just use the initial version of this patch (the
> returning < 2 check in sscanf() function).
>
> - (3/3): New patch that fixes removing device services from
> "primary" file by passing a key with the new
> storage format in delete_device_service(). Note
> also that this function was modified to only treat
> the primary case - since we have old storage format
> still being used in the other LE-related storage
> files. So, it will need to be modified in order to
> delete device services of other files (characteristics,
> ccc, etc) on other series.
>
>
> Claudio Takahasi (1):
> core: Fix creating device from "primary" file
>
> Paulo Alcantara (2):
> storage: Store BLE address type in "primary" file
> core: Fix removing device services from "primary" file
>
> src/adapter.c | 10 +++++++---
> src/device.c | 4 ++--
> src/storage.c | 45 ++++++++++++++++++++++++++-------------------
> src/storage.h | 8 +++++---
> 4 files changed, 40 insertions(+), 27 deletions(-)
All three patches have been applied. Thanks.
Johan
The BLE address type needs to be passed as parameter in
delete_device_service() function in order to remove a device from
"primary" file. This is why the entries in "primary" file will be using
the new storage format.
---
src/device.c | 2 +-
src/storage.c | 26 +++++++++++++++-----------
src/storage.h | 3 ++-
3 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/src/device.c b/src/device.c
index fd13bc3..7cdd025 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1169,7 +1169,7 @@ static void device_remove_stored(struct btd_device *device)
delete_entry(&src, "profiles", addr);
delete_entry(&src, "trusts", addr);
delete_all_records(&src, &device->bdaddr);
- delete_device_service(&src, &device->bdaddr);
+ delete_device_service(&src, &device->bdaddr, device->bdaddr_type);
if (device->blocked)
device_unblock(conn, device, TRUE, FALSE);
diff --git a/src/storage.c b/src/storage.c
index 33934fb..973d545 100644
--- a/src/storage.c
+++ b/src/storage.c
@@ -1206,27 +1206,31 @@ done:
g_slist_free_full(match.keys, g_free);
}
-int delete_device_service(const bdaddr_t *sba, const bdaddr_t *dba)
+int delete_device_service(const bdaddr_t *sba, const bdaddr_t *dba,
+ uint8_t bdaddr_type)
{
- char filename[PATH_MAX + 1], address[18];
+ char filename[PATH_MAX + 1], key[20];
- memset(address, 0, sizeof(address));
- ba2str(dba, address);
+ memset(key, 0, sizeof(key));
+ ba2str(dba, key);
- /* Deleting all characteristics of a given address */
+ /* Deleting all characteristics of a given key */
create_filename(filename, PATH_MAX, sba, "characteristic");
- delete_by_pattern(filename, address);
+ delete_by_pattern(filename, key);
- /* Deleting all attributes values of a given address */
+ /* Deleting all attributes values of a given key */
create_filename(filename, PATH_MAX, sba, "attributes");
- delete_by_pattern(filename, address);
+ delete_by_pattern(filename, key);
- /* Deleting all CCC values of a given address */
+ /* Deleting all CCC values of a given key */
create_filename(filename, PATH_MAX, sba, "ccc");
- delete_by_pattern(filename, address);
+ delete_by_pattern(filename, key);
+
+ sprintf(&key[17], "#%hhu", bdaddr_type);
create_filename(filename, PATH_MAX, sba, "primary");
- return textfile_del(filename, address);
+
+ return textfile_del(filename, key);
}
char *read_device_services(const bdaddr_t *sba, const bdaddr_t *dba,
diff --git a/src/storage.h b/src/storage.h
index 745aff8..d00976f 100644
--- a/src/storage.h
+++ b/src/storage.h
@@ -77,7 +77,8 @@ int write_blocked(const bdaddr_t *local, const bdaddr_t *remote,
gboolean blocked);
int write_device_services(const bdaddr_t *sba, const bdaddr_t *dba,
uint8_t bdaddr_type, const char *services);
-int delete_device_service(const bdaddr_t *sba, const bdaddr_t *dba);
+int delete_device_service(const bdaddr_t *sba, const bdaddr_t *dba,
+ uint8_t bdaddr_type);
char *read_device_services(const bdaddr_t *sba, const bdaddr_t *dba,
uint8_t bdaddr_type);
int write_device_characteristics(const bdaddr_t *sba, const bdaddr_t *dba,
--
1.7.7.6
From: Claudio Takahasi <[email protected]>
This patch removes the hard-coded address type for the BLE device
created from the storage.
---
src/adapter.c | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index dafe595..18dd5b6 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1940,13 +1940,17 @@ static void create_stored_device_from_primary(char *key, char *value,
struct btd_adapter *adapter = user_data;
struct btd_device *device;
GSList *services, *uuids, *l;
+ char address[18];
+ uint8_t bdaddr_type;
+
+ if (sscanf(key, "%17s#%hhu", address, &bdaddr_type) < 2)
+ return;
if (g_slist_find_custom(adapter->devices,
- key, (GCompareFunc) device_address_cmp))
+ address, (GCompareFunc) device_address_cmp))
return;
- /* FIXME: Get the correct LE addr type (public/random) */
- device = device_create(connection, adapter, key, BDADDR_LE_PUBLIC);
+ device = device_create(connection, adapter, address, bdaddr_type);
if (!device)
return;
--
1.7.7.6
BLE addressing types can be either public or random so the entries in
the "primary" file did not contain enough information to distinguish
which addressing type it's supposed to be (LE public or LE random).
Entries will now contain both BLE address number and BLE address type
as a single key in every entry in the file.
---
src/device.c | 2 +-
src/storage.c | 19 +++++++++++--------
src/storage.h | 5 +++--
3 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/src/device.c b/src/device.c
index 2695b16..fd13bc3 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1759,7 +1759,7 @@ static void store_services(struct btd_device *device)
adapter_get_address(adapter, &sba);
device_get_address(device, &dba, NULL);
- write_device_services(&sba, &dba, str);
+ write_device_services(&sba, &dba, device->bdaddr_type, str);
g_free(str);
}
diff --git a/src/storage.c b/src/storage.c
index 14e5ac3..33934fb 100644
--- a/src/storage.c
+++ b/src/storage.c
@@ -1162,17 +1162,18 @@ int write_blocked(const bdaddr_t *local, const bdaddr_t *remote,
}
int write_device_services(const bdaddr_t *sba, const bdaddr_t *dba,
- const char *services)
+ uint8_t bdaddr_type, const char *services)
{
- char filename[PATH_MAX + 1], addr[18];
+ char filename[PATH_MAX + 1], key[20];
create_filename(filename, PATH_MAX, sba, "primary");
create_file(filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
- ba2str(dba, addr);
+ ba2str(dba, key);
+ sprintf(&key[17], "#%hhu", bdaddr_type);
- return textfile_put(filename, addr, services);
+ return textfile_put(filename, key, services);
}
static void filter_keys(char *key, char *value, void *data)
@@ -1228,15 +1229,17 @@ int delete_device_service(const bdaddr_t *sba, const bdaddr_t *dba)
return textfile_del(filename, address);
}
-char *read_device_services(const bdaddr_t *sba, const bdaddr_t *dba)
+char *read_device_services(const bdaddr_t *sba, const bdaddr_t *dba,
+ uint8_t bdaddr_type)
{
- char filename[PATH_MAX + 1], addr[18];
+ char filename[PATH_MAX + 1], key[20];
create_filename(filename, PATH_MAX, sba, "primary");
- ba2str(dba, addr);
+ ba2str(dba, key);
+ sprintf(&key[17], "#%hhu", bdaddr_type);
- return textfile_caseget(filename, addr);
+ return textfile_caseget(filename, key);
}
int write_device_characteristics(const bdaddr_t *sba, const bdaddr_t *dba,
diff --git a/src/storage.h b/src/storage.h
index 6255ae8..745aff8 100644
--- a/src/storage.h
+++ b/src/storage.h
@@ -76,9 +76,10 @@ gboolean read_blocked(const bdaddr_t *local, const bdaddr_t *remote);
int write_blocked(const bdaddr_t *local, const bdaddr_t *remote,
gboolean blocked);
int write_device_services(const bdaddr_t *sba, const bdaddr_t *dba,
- const char *services);
+ uint8_t bdaddr_type, const char *services);
int delete_device_service(const bdaddr_t *sba, const bdaddr_t *dba);
-char *read_device_services(const bdaddr_t *sba, const bdaddr_t *dba);
+char *read_device_services(const bdaddr_t *sba, const bdaddr_t *dba,
+ uint8_t bdaddr_type);
int write_device_characteristics(const bdaddr_t *sba, const bdaddr_t *dba,
uint16_t handle, const char *chars);
char *read_device_characteristics(const bdaddr_t *sba, const bdaddr_t *dba,
--
1.7.7.6
BLE devices have two different address types: public or
random. So we need to provide this address type on keys
in the "primary" file to avoid duplicate BLE address types
around the BlueZ.
This series changes the storage to store the address type
together with the device address.
Hemant Gupta has been working on this on "Store LE device address
type with primary list" but his patch hasn't been accepted and
he didn't send a new reviewed version of that patch.
I have patches for the other storage files as well, I'll
send them soon. There is a potential problem in
device_address_cmp() function which checks the address only.
I am investigating how to fix it to compare address and type
without breaking the current code.
v3's changes:
- (1/3): Remove unnecessary comment about the new storage
format and code that leads with the old storage
format.
- (2/3): Just use the initial version of this patch (the
returning < 2 check in sscanf() function).
- (3/3): New patch that fixes removing device services from
"primary" file by passing a key with the new
storage format in delete_device_service(). Note
also that this function was modified to only treat
the primary case - since we have old storage format
still being used in the other LE-related storage
files. So, it will need to be modified in order to
delete device services of other files (characteristics,
ccc, etc) on other series.
Claudio Takahasi (1):
core: Fix creating device from "primary" file
Paulo Alcantara (2):
storage: Store BLE address type in "primary" file
core: Fix removing device services from "primary" file
src/adapter.c | 10 +++++++---
src/device.c | 4 ++--
src/storage.c | 45 ++++++++++++++++++++++++++-------------------
src/storage.h | 8 +++++---
4 files changed, 40 insertions(+), 27 deletions(-)
--
1.7.7.6
Hi Johan,
On Tue, May 22, 2012 at 5:00 AM, Johan Hedberg <[email protected]> wrote:
> Hi,
>
> On Mon, May 21, 2012, Paulo Alcantara wrote:
>> From: Claudio Takahasi <[email protected]>
>>
>> This patch removes the hard-coded address type for the BLE device
>> created from the storage.
>> ---
>> src/adapter.c | 10 +++++++---
>> 1 files changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/adapter.c b/src/adapter.c
>> index dafe595..1ca21e6 100644
>> --- a/src/adapter.c
>> +++ b/src/adapter.c
>> @@ -1940,13 +1940,17 @@ static void create_stored_device_from_primary(char *key, char *value,
>> struct btd_adapter *adapter = user_data;
>> struct btd_device *device;
>> GSList *services, *uuids, *l;
>> + char address[18];
>> + uint8_t bdaddr_type;
>> +
>> + if (sscanf(key, "%17s#%hhu", address, &bdaddr_type) < 2)
>> + bdaddr_type = BDADDR_LE_PUBLIC;
>
> That's not safe. What if sscanf returns 0 or a negative value? In that
> case the address variable will remain uninitialized (which is why you
> should have just followed my suggestion of testing for < 1 and returning
> in such a case).
IMO, it is not necessary to add extra verification since only
bluetoothd changes this file.
It will be an unnecessary overhead.
>
> Thinking more about this situation I'm not sure if it's any better to
> allow creation of old entries since you won't be able to remove them
> anyway: the remove code looks for a bdaddr#type key which won't exist
> and adding code to look for both types of keys is just bloating the code
> base for a minor benefit.
>
> So maybe your initial patch of failing in the case of sscanf returning < 2
> is good enough after all.
>
> Johan
As you mentioned in the reply for "v2 1/2" of this patch series, LE
was enabled by default recently, and there isn't releases supporting
officially LE. Keep the compatibility with the old format will not
give a valuable benefit.
We will fix the other issues that you pointed, and send the patch series again.
Claudio.
Hi Johan,
From: Johan Hedberg <[email protected]>
Date: Tue, 22 May 2012 11:05:36 +0300
> > + /* New format: address#type */
> > + ba2str(dba, key);
> > + sprintf(&key[17], "#%hhu", bdaddr_type);
>
> Since BlueZ 4.100 will be the first user space version being able to use
> mgmtops with a kernel where mgmt is enabled by default (3.4) I don't
> think we need to worry about old format vs new format, i.e. the code
> comment above is unnecessary.
Ok. When we were implemeting this, our first thought was to just ignore the old
storage formats and then use the new storage one.
So, later we decided to keep backward compatibily with the old storage format
since one might not want to delete his old entries so that the patch wouldn't
work with this new storage format (the old keys would never be found).
I'll remove the comment above. Thanks for pointing it out.
>
> > + /* New format: address#type */
> > + ba2str(dba, key);
> > + sprintf(&key[17], "#%hhu", bdaddr_type);
> > +
> > + str = textfile_caseget(filename, key);
> > + if (str != NULL)
> > + return str;
> > +
> > + /* Old format: address only */
> > + key[17] = '\0';
> > +
> > + return textfile_caseget(filename, key);
>
> Same thing here. I think we can leave out any code that tries to work
> with the old storage format since we can consider LE support as
> experimental/unstable in all previous user space releases.
Fair enough. I'll remove the comment above, as well the code that leads with the
old storage format. Thanks again!
Paulo
Hi Paulo,
On Mon, May 21, 2012, Paulo Alcantara wrote:
> + /* New format: address#type */
> + ba2str(dba, key);
> + sprintf(&key[17], "#%hhu", bdaddr_type);
Since BlueZ 4.100 will be the first user space version being able to use
mgmtops with a kernel where mgmt is enabled by default (3.4) I don't
think we need to worry about old format vs new format, i.e. the code
comment above is unnecessary.
> + /* New format: address#type */
> + ba2str(dba, key);
> + sprintf(&key[17], "#%hhu", bdaddr_type);
> +
> + str = textfile_caseget(filename, key);
> + if (str != NULL)
> + return str;
> +
> + /* Old format: address only */
> + key[17] = '\0';
> +
> + return textfile_caseget(filename, key);
Same thing here. I think we can leave out any code that tries to work
with the old storage format since we can consider LE support as
experimental/unstable in all previous user space releases.
Johan
Hi,
On Mon, May 21, 2012, Paulo Alcantara wrote:
> From: Claudio Takahasi <[email protected]>
>
> This patch removes the hard-coded address type for the BLE device
> created from the storage.
> ---
> src/adapter.c | 10 +++++++---
> 1 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index dafe595..1ca21e6 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -1940,13 +1940,17 @@ static void create_stored_device_from_primary(char *key, char *value,
> struct btd_adapter *adapter = user_data;
> struct btd_device *device;
> GSList *services, *uuids, *l;
> + char address[18];
> + uint8_t bdaddr_type;
> +
> + if (sscanf(key, "%17s#%hhu", address, &bdaddr_type) < 2)
> + bdaddr_type = BDADDR_LE_PUBLIC;
That's not safe. What if sscanf returns 0 or a negative value? In that
case the address variable will remain uninitialized (which is why you
should have just followed my suggestion of testing for < 1 and returning
in such a case).
Thinking more about this situation I'm not sure if it's any better to
allow creation of old entries since you won't be able to remove them
anyway: the remove code looks for a bdaddr#type key which won't exist
and adding code to look for both types of keys is just bloating the code
base for a minor benefit.
So maybe your initial patch of failing in the case of sscanf returning < 2
is good enough after all.
Johan
Hi Johan,
From: Johan Hedberg <[email protected]>
Date: Sat, 19 May 2012 09:43:41 +0300
> Hi,
>
> On Fri, May 18, 2012, Paulo Alcantara wrote:
> > This patch removes the hard-coded address type for the BLE device
> > created from the storage.
> > ---
> > src/adapter.c | 10 +++++++---
> > 1 files changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/adapter.c b/src/adapter.c
> > index dafe595..8d86ca8 100644
> > --- a/src/adapter.c
> > +++ b/src/adapter.c
> > @@ -1940,13 +1940,17 @@ static void create_stored_device_from_primary(char *key, char *value,
> > struct btd_adapter *adapter = user_data;
> > struct btd_device *device;
> > GSList *services, *uuids, *l;
> > + char address[18];
> > + uint8_t bdaddr_type;
> > +
> > + if (sscanf(key, "%17s#%hhu", address, &bdaddr_type) != 2)
> > + return;
>
> What about entries created by older bluetoothd versions? Will they be
> stuck in the storage forever without a way to remove them through the
> usual API? Instead of checking for != 2 maybe the check should instead
> be < 1 and bdaddr_type be pre-initialized to BDADDR_LE_PUBLIC.
I agree with you that we must keep backward compatibility here with older
entries. The check here would be "< 2" (in case we only have device address and
not its address type type) instead, and then pre-initialize bdaddr_type to
BDADDR_LE_PUBLIC.
Fixed it, thanks.
Paulo
Hi Johan,
From: Johan Hedberg <[email protected]>
Date: Sat, 19 May 2012 09:41:24 +0300
> > + /* New format: address#type */
> > + ba2str(dba, key);
> > + key[17] = '#';
> > + key[18] = ba_type2char(bdaddr_type);
>
> Wouldn't sprintf(&key[17], "#%hhu", bdaddr_type); be much simpler?
Agreed. Fixed it.
> > + /* New format: address#type */
> > + ba2str(dba, key);
> > + key[17] = '#';
> > + key[18] = ba_type2char(bdaddr_type);
>
> Same here.
Fixed it too.
Paulo
From: Claudio Takahasi <[email protected]>
This patch removes the hard-coded address type for the BLE device
created from the storage.
---
src/adapter.c | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index dafe595..1ca21e6 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1940,13 +1940,17 @@ static void create_stored_device_from_primary(char *key, char *value,
struct btd_adapter *adapter = user_data;
struct btd_device *device;
GSList *services, *uuids, *l;
+ char address[18];
+ uint8_t bdaddr_type;
+
+ if (sscanf(key, "%17s#%hhu", address, &bdaddr_type) < 2)
+ bdaddr_type = BDADDR_LE_PUBLIC;
if (g_slist_find_custom(adapter->devices,
- key, (GCompareFunc) device_address_cmp))
+ address, (GCompareFunc) device_address_cmp))
return;
- /* FIXME: Get the correct LE addr type (public/random) */
- device = device_create(connection, adapter, key, BDADDR_LE_PUBLIC);
+ device = device_create(connection, adapter, address, bdaddr_type);
if (!device)
return;
--
1.7.7.6
BLE addressing types can be either public or random so the entries in
the "primary" file did not contain enough information to distinguish
which addressing type it's supposed to be (LE public or LE random).
Entries will now contain both BLE address number and BLE address type
as a single key in every entry in the file.
---
src/device.c | 2 +-
src/storage.c | 32 ++++++++++++++++++++++++--------
src/storage.h | 5 +++--
3 files changed, 28 insertions(+), 11 deletions(-)
diff --git a/src/device.c b/src/device.c
index b180579..2bb428c 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1759,7 +1759,7 @@ static void store_services(struct btd_device *device)
adapter_get_address(adapter, &sba);
device_get_address(device, &dba, NULL);
- write_device_services(&sba, &dba, str);
+ write_device_services(&sba, &dba, device->bdaddr_type, str);
g_free(str);
}
diff --git a/src/storage.c b/src/storage.c
index 14e5ac3..5603cb8 100644
--- a/src/storage.c
+++ b/src/storage.c
@@ -1162,17 +1162,21 @@ int write_blocked(const bdaddr_t *local, const bdaddr_t *remote,
}
int write_device_services(const bdaddr_t *sba, const bdaddr_t *dba,
- const char *services)
+ uint8_t bdaddr_type, const char *services)
{
- char filename[PATH_MAX + 1], addr[18];
+ char filename[PATH_MAX + 1], key[20];
create_filename(filename, PATH_MAX, sba, "primary");
create_file(filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
- ba2str(dba, addr);
+ memset(key, 0, sizeof(key));
+
+ /* New format: address#type */
+ ba2str(dba, key);
+ sprintf(&key[17], "#%hhu", bdaddr_type);
- return textfile_put(filename, addr, services);
+ return textfile_put(filename, key, services);
}
static void filter_keys(char *key, char *value, void *data)
@@ -1228,15 +1232,27 @@ int delete_device_service(const bdaddr_t *sba, const bdaddr_t *dba)
return textfile_del(filename, address);
}
-char *read_device_services(const bdaddr_t *sba, const bdaddr_t *dba)
+char *read_device_services(const bdaddr_t *sba, const bdaddr_t *dba,
+ uint8_t bdaddr_type)
{
- char filename[PATH_MAX + 1], addr[18];
+ char filename[PATH_MAX + 1], key[20], *str;
create_filename(filename, PATH_MAX, sba, "primary");
- ba2str(dba, addr);
+ memset(key, 0, sizeof(key));
- return textfile_caseget(filename, addr);
+ /* New format: address#type */
+ ba2str(dba, key);
+ sprintf(&key[17], "#%hhu", bdaddr_type);
+
+ str = textfile_caseget(filename, key);
+ if (str != NULL)
+ return str;
+
+ /* Old format: address only */
+ key[17] = '\0';
+
+ return textfile_caseget(filename, key);
}
int write_device_characteristics(const bdaddr_t *sba, const bdaddr_t *dba,
diff --git a/src/storage.h b/src/storage.h
index 6255ae8..745aff8 100644
--- a/src/storage.h
+++ b/src/storage.h
@@ -76,9 +76,10 @@ gboolean read_blocked(const bdaddr_t *local, const bdaddr_t *remote);
int write_blocked(const bdaddr_t *local, const bdaddr_t *remote,
gboolean blocked);
int write_device_services(const bdaddr_t *sba, const bdaddr_t *dba,
- const char *services);
+ uint8_t bdaddr_type, const char *services);
int delete_device_service(const bdaddr_t *sba, const bdaddr_t *dba);
-char *read_device_services(const bdaddr_t *sba, const bdaddr_t *dba);
+char *read_device_services(const bdaddr_t *sba, const bdaddr_t *dba,
+ uint8_t bdaddr_type);
int write_device_characteristics(const bdaddr_t *sba, const bdaddr_t *dba,
uint16_t handle, const char *chars);
char *read_device_characteristics(const bdaddr_t *sba, const bdaddr_t *dba,
--
1.7.7.6
BLE devices have two different address types: public or
random. So we need to provide this address type on keys
in the "primary" file to avoid duplicate BLE address types
around the BlueZ.
This series changes the storage to store the address type
together with the device address.
Hemant Gupta has been working on this on "Store LE device address
type with primary list" but his patch hasn't been accepted and
he didn't send a new reviewed version of that patch.
I have patches for the other storage files as well, I'll
send them soon. There is a potential problem in
device_address_cmp() function which checks the address only.
I am investigating how to fix it to compare address and type
without breaking the current code.
v2's changes:
- Use sprintf() instead of setting values by indexing
strings.
- Keep backward compatibility with entries created by
older bluetoothd versions.
Claudio Takahasi (1):
core: Fix creating device from "primary" file
Paulo Alcantara (1):
storage: Store BLE address type in "primary" file
src/adapter.c | 10 +++++++---
src/device.c | 2 +-
src/storage.c | 32 ++++++++++++++++++++++++--------
src/storage.h | 5 +++--
4 files changed, 35 insertions(+), 14 deletions(-)
--
1.7.7.6
Hi,
On Fri, May 18, 2012, Paulo Alcantara wrote:
> This patch removes the hard-coded address type for the BLE device
> created from the storage.
> ---
> src/adapter.c | 10 +++++++---
> 1 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index dafe595..8d86ca8 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -1940,13 +1940,17 @@ static void create_stored_device_from_primary(char *key, char *value,
> struct btd_adapter *adapter = user_data;
> struct btd_device *device;
> GSList *services, *uuids, *l;
> + char address[18];
> + uint8_t bdaddr_type;
> +
> + if (sscanf(key, "%17s#%hhu", address, &bdaddr_type) != 2)
> + return;
What about entries created by older bluetoothd versions? Will they be
stuck in the storage forever without a way to remove them through the
usual API? Instead of checking for != 2 maybe the check should instead
be < 1 and bdaddr_type be pre-initialized to BDADDR_LE_PUBLIC.
Johan
Hi Paulo,
On Fri, May 18, 2012, Paulo Alcantara wrote:
> +static inline char ba_type2char(uint8_t bdaddr_type)
> +{
> + switch (bdaddr_type) {
> + case BDADDR_LE_PUBLIC:
> + return '1';
> + case BDADDR_LE_RANDOM:
> + return '2';
> + case BDADDR_BREDR:
> + default:
> + return '0';
> + }
> +}
> +
> int read_device_alias(const char *src, const char *dst, char *alias, size_t size)
> {
> char filename[PATH_MAX + 1], *tmp;
> @@ -1162,17 +1175,22 @@ int write_blocked(const bdaddr_t *local, const bdaddr_t *remote,
> }
>
> int write_device_services(const bdaddr_t *sba, const bdaddr_t *dba,
> - const char *services)
> + uint8_t bdaddr_type, const char *services)
> {
> - char filename[PATH_MAX + 1], addr[18];
> + char filename[PATH_MAX + 1], key[20];
>
> create_filename(filename, PATH_MAX, sba, "primary");
>
> create_file(filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
>
> - ba2str(dba, addr);
> + memset(key, 0, sizeof(key));
>
> - return textfile_put(filename, addr, services);
> + /* New format: address#type */
> + ba2str(dba, key);
> + key[17] = '#';
> + key[18] = ba_type2char(bdaddr_type);
Wouldn't sprintf(&key[17], "#%hhu", bdaddr_type); be much simpler?
> +char *read_device_services(const bdaddr_t *sba, const bdaddr_t *dba,
> + uint8_t bdaddr_type)
> {
> - char filename[PATH_MAX + 1], addr[18];
> + char filename[PATH_MAX + 1], key[20];
>
> create_filename(filename, PATH_MAX, sba, "primary");
>
> - ba2str(dba, addr);
> + memset(key, 0, sizeof(key));
>
> - return textfile_caseget(filename, addr);
> + /* New format: address#type */
> + ba2str(dba, key);
> + key[17] = '#';
> + key[18] = ba_type2char(bdaddr_type);
Same here.
Johan
From: Claudio Takahasi <[email protected]>
This patch removes the hard-coded address type for the BLE device
created from the storage.
---
src/adapter.c | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index dafe595..8d86ca8 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1940,13 +1940,17 @@ static void create_stored_device_from_primary(char *key, char *value,
struct btd_adapter *adapter = user_data;
struct btd_device *device;
GSList *services, *uuids, *l;
+ char address[18];
+ uint8_t bdaddr_type;
+
+ if (sscanf(key, "%17s#%hhu", address, &bdaddr_type) != 2)
+ return;
if (g_slist_find_custom(adapter->devices,
- key, (GCompareFunc) device_address_cmp))
+ address, (GCompareFunc) device_address_cmp))
return;
- /* FIXME: Get the correct LE addr type (public/random) */
- device = device_create(connection, adapter, key, BDADDR_LE_PUBLIC);
+ device = device_create(connection, adapter, address, bdaddr_type);
if (!device)
return;
--
1.7.7.6
BLE addressing types can be either public or random so the entries in
the "primary" file did not contain enough information to distinguish
which addressing type it's supposed to be (LE public or LE random).
Entries will now contain both BLE address number and BLE address type
as a single key in every entry in the file.
---
src/device.c | 2 +-
src/storage.c | 40 ++++++++++++++++++++++++++++++++--------
src/storage.h | 5 +++--
3 files changed, 36 insertions(+), 11 deletions(-)
diff --git a/src/device.c b/src/device.c
index 2695b16..fd13bc3 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1759,7 +1759,7 @@ static void store_services(struct btd_device *device)
adapter_get_address(adapter, &sba);
device_get_address(device, &dba, NULL);
- write_device_services(&sba, &dba, str);
+ write_device_services(&sba, &dba, device->bdaddr_type, str);
g_free(str);
}
diff --git a/src/storage.c b/src/storage.c
index 14e5ac3..815060e 100644
--- a/src/storage.c
+++ b/src/storage.c
@@ -61,6 +61,19 @@ static inline int create_filename(char *buf, size_t size,
return create_name(buf, size, STORAGEDIR, addr, name);
}
+static inline char ba_type2char(uint8_t bdaddr_type)
+{
+ switch (bdaddr_type) {
+ case BDADDR_LE_PUBLIC:
+ return '1';
+ case BDADDR_LE_RANDOM:
+ return '2';
+ case BDADDR_BREDR:
+ default:
+ return '0';
+ }
+}
+
int read_device_alias(const char *src, const char *dst, char *alias, size_t size)
{
char filename[PATH_MAX + 1], *tmp;
@@ -1162,17 +1175,22 @@ int write_blocked(const bdaddr_t *local, const bdaddr_t *remote,
}
int write_device_services(const bdaddr_t *sba, const bdaddr_t *dba,
- const char *services)
+ uint8_t bdaddr_type, const char *services)
{
- char filename[PATH_MAX + 1], addr[18];
+ char filename[PATH_MAX + 1], key[20];
create_filename(filename, PATH_MAX, sba, "primary");
create_file(filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
- ba2str(dba, addr);
+ memset(key, 0, sizeof(key));
- return textfile_put(filename, addr, services);
+ /* New format: address#type */
+ ba2str(dba, key);
+ key[17] = '#';
+ key[18] = ba_type2char(bdaddr_type);
+
+ return textfile_put(filename, key, services);
}
static void filter_keys(char *key, char *value, void *data)
@@ -1228,15 +1246,21 @@ int delete_device_service(const bdaddr_t *sba, const bdaddr_t *dba)
return textfile_del(filename, address);
}
-char *read_device_services(const bdaddr_t *sba, const bdaddr_t *dba)
+char *read_device_services(const bdaddr_t *sba, const bdaddr_t *dba,
+ uint8_t bdaddr_type)
{
- char filename[PATH_MAX + 1], addr[18];
+ char filename[PATH_MAX + 1], key[20];
create_filename(filename, PATH_MAX, sba, "primary");
- ba2str(dba, addr);
+ memset(key, 0, sizeof(key));
- return textfile_caseget(filename, addr);
+ /* New format: address#type */
+ ba2str(dba, key);
+ key[17] = '#';
+ key[18] = ba_type2char(bdaddr_type);
+
+ return textfile_caseget(filename, key);
}
int write_device_characteristics(const bdaddr_t *sba, const bdaddr_t *dba,
diff --git a/src/storage.h b/src/storage.h
index 6255ae8..745aff8 100644
--- a/src/storage.h
+++ b/src/storage.h
@@ -76,9 +76,10 @@ gboolean read_blocked(const bdaddr_t *local, const bdaddr_t *remote);
int write_blocked(const bdaddr_t *local, const bdaddr_t *remote,
gboolean blocked);
int write_device_services(const bdaddr_t *sba, const bdaddr_t *dba,
- const char *services);
+ uint8_t bdaddr_type, const char *services);
int delete_device_service(const bdaddr_t *sba, const bdaddr_t *dba);
-char *read_device_services(const bdaddr_t *sba, const bdaddr_t *dba);
+char *read_device_services(const bdaddr_t *sba, const bdaddr_t *dba,
+ uint8_t bdaddr_type);
int write_device_characteristics(const bdaddr_t *sba, const bdaddr_t *dba,
uint16_t handle, const char *chars);
char *read_device_characteristics(const bdaddr_t *sba, const bdaddr_t *dba,
--
1.7.7.6