[PATCH] adapter: Fix compilation on some architectures

[PATCH] adapter: Fix compilation on some architectures

[Szymon Janc]

plain char may be treated as signed or unsigned depending on compiler
(as oposite to other integer types). Use signed char explicitly when
signed type is needed.

This fix following compilation error on ARM:

  CC     src/bluetoothd-adapter.o
src/adapter.c: In function ‘convert_entry’:
src/adapter.c:2710:2: error: comparison is always true due to limited
	range of data type [-Werror=type-limits]
cc1: all warnings being treated as errors
make[1]: *** [src/bluetoothd-adapter.o] Error 1
---
 src/adapter.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/adapter.c b/src/adapter.c
index 3c5d277..0cbce78 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2673,7 +2673,7 @@ static void convert_ltk_entry(GKeyFile *key_file, void *value)
 static void convert_entry(char *key, char *value, void *user_data)
 {
 	struct device_converter *converter = user_data;
-	char device_type = -1;
+	signed char device_type = -1;
 	char filename[PATH_MAX + 1];
 	GKeyFile *key_file;
 	char *data;
-- 
1.7.9.5

Re: [PATCH] adapter: Fix compilation on some architectures

[Johan Hedberg]

Hi Szymon,

On Wed, Dec 05, 2012, Szymon Janc wrote:
> plain char may be treated as signed or unsigned depending on compiler
> (as oposite to other integer types). Use signed char explicitly when
> signed type is needed.
> 
> This fix following compilation error on ARM:
> 
>   CC     src/bluetoothd-adapter.o
> src/adapter.c: In function ‘convert_entry’:
> src/adapter.c:2710:2: error: comparison is always true due to limited
> 	range of data type [-Werror=type-limits]
> cc1: all warnings being treated as errors
> make[1]: *** [src/bluetoothd-adapter.o] Error 1
> ---
>  src/adapter.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/adapter.c b/src/adapter.c
> index 3c5d277..0cbce78 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -2673,7 +2673,7 @@ static void convert_ltk_entry(GKeyFile *key_file, void *value)
>  static void convert_entry(char *key, char *value, void *user_data)
>  {
>  	struct device_converter *converter = user_data;
> -	char device_type = -1;
> +	signed char device_type = -1;
>  	char filename[PATH_MAX + 1];
>  	GKeyFile *key_file;
>  	char *data;

Couldn't we just initialize the value to BDADDR_BREDR or maybe use an
int for it. And the naming is confusing as the value contains an address
type and not a device type.

Johan

Re: [PATCH] adapter: Fix compilation on some architectures

[Szymon Janc]

On Wednesday 05 of December 2012 12:29:58 Johan Hedberg wrote:
> Hi Szymon,

Hi Johan,
> 
> On Wed, Dec 05, 2012, Szymon Janc wrote:
> > plain char may be treated as signed or unsigned depending on compiler
> > (as oposite to other integer types). Use signed char explicitly when
> > signed type is needed.
> > 
> > This fix following compilation error on ARM:
> > 
> >   CC     src/bluetoothd-adapter.o
> > src/adapter.c: In function ‘convert_entry’:
> > src/adapter.c:2710:2: error: comparison is always true due to limited
> > 	range of data type [-Werror=type-limits]
> > cc1: all warnings being treated as errors
> > make[1]: *** [src/bluetoothd-adapter.o] Error 1
> > ---
> >  src/adapter.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/adapter.c b/src/adapter.c
> > index 3c5d277..0cbce78 100644
> > --- a/src/adapter.c
> > +++ b/src/adapter.c
> > @@ -2673,7 +2673,7 @@ static void convert_ltk_entry(GKeyFile *key_file, void *value)
> >  static void convert_entry(char *key, char *value, void *user_data)
> >  {
> >  	struct device_converter *converter = user_data;
> > -	char device_type = -1;
> > +	signed char device_type = -1;
> >  	char filename[PATH_MAX + 1];
> >  	GKeyFile *key_file;
> >  	char *data;
> 
> Couldn't we just initialize the value to BDADDR_BREDR or maybe use an
> int for it. And the naming is confusing as the value contains an address
> type and not a device type.

Initializing it to BDADDR_BREDR wouldn't fix compilation as problem is in
comparing >=0 when type is unsigned (and would change function flow as now
device_set_type is called only if device_type is >=0)...  I'm not sure what
was decided about storage changes so didn't wanted to fiddle with that.

I agree that name is a bit confusing but I guess this is just a consequence
of set_device_type() function name.

So, if it is OK to always fall back to BDADDR_BREDR I can resent V2 with that
change, else I can sent V2 with use of int if you prefer it over signed char.

-- 
BR
Szymon Janc

Re: [PATCH] adapter: Fix compilation on some architectures

[Johan Hedberg]

Hi Szymon,

On Wed, Dec 05, 2012, Szymon Janc wrote:
> > > --- a/src/adapter.c
> > > +++ b/src/adapter.c
> > > @@ -2673,7 +2673,7 @@ static void convert_ltk_entry(GKeyFile *key_file, void *value)
> > >  static void convert_entry(char *key, char *value, void *user_data)
> > >  {
> > >  	struct device_converter *converter = user_data;
> > > -	char device_type = -1;
> > > +	signed char device_type = -1;
> > >  	char filename[PATH_MAX + 1];
> > >  	GKeyFile *key_file;
> > >  	char *data;
> > 
> > Couldn't we just initialize the value to BDADDR_BREDR or maybe use an
> > int for it. And the naming is confusing as the value contains an address
> > type and not a device type.
> 
> Initializing it to BDADDR_BREDR wouldn't fix compilation as problem is in
> comparing >=0 when type is unsigned (and would change function flow as now
> device_set_type is called only if device_type is >=0)...  I'm not sure what
> was decided about storage changes so didn't wanted to fiddle with that.

I meant that you'd also then remove the test for >= 0 and always write
the type (defaulting to BR/EDR)

> So, if it is OK to always fall back to BDADDR_BREDR I can resent V2 with that
> change, else I can sent V2 with use of int if you prefer it over signed char.

I'd just default to BDADDR_BREDR. That's what entries without an
explicit type are going to be anyway since older BlueZ versions didn't
support LE.

Johan

[PATCH] adapter: Always write address type when converting to new storage

[Szymon Janc]

When address type is not present in legacy storage assume BDADDR_BREDR
type and store it.

This also fix following compilation error on ARM:

  CC     src/bluetoothd-adapter.o
src/adapter.c: In function ‘convert_entry’:
src/adapter.c:2710:2: error: comparison is always true due to limited
	range of data type [-Werror=type-limits]
cc1: all warnings being treated as errors
make[1]: *** [src/bluetoothd-adapter.o] Error 1
---
 src/adapter.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 3c5d277..9bef239 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2673,7 +2673,7 @@ static void convert_ltk_entry(GKeyFile *key_file, void *value)
 static void convert_entry(char *key, char *value, void *user_data)
 {
 	struct device_converter *converter = user_data;
-	char device_type = -1;
+	char type = BDADDR_BREDR;
 	char filename[PATH_MAX + 1];
 	GKeyFile *key_file;
 	char *data;
@@ -2681,7 +2681,7 @@ static void convert_entry(char *key, char *value, void *user_data)
 
 	if (key[17] == '#') {
 		key[17] = '\0';
-		device_type = key[18] - '0';
+		type = key[18] - '0';
 	}
 
 	if (bachk(key) != 0)
@@ -2707,8 +2707,7 @@ static void convert_entry(char *key, char *value, void *user_data)
 	key_file = g_key_file_new();
 	g_key_file_load_from_file(key_file, filename, 0, NULL);
 
-	if (device_type >= 0)
-		set_device_type(key_file, device_type);
+	set_device_type(key_file, type);
 
 	converter->cb(key_file, value);
 
-- 
1.7.9.5

Re: [PATCH] adapter: Always write address type when converting to new storage

[Johan Hedberg]

Hi Szymon,

On Wed, Dec 05, 2012, Szymon Janc wrote:
> When address type is not present in legacy storage assume BDADDR_BREDR
> type and store it.
> 
> This also fix following compilation error on ARM:
> 
>   CC     src/bluetoothd-adapter.o
> src/adapter.c: In function ‘convert_entry’:
> src/adapter.c:2710:2: error: comparison is always true due to limited
> 	range of data type [-Werror=type-limits]
> cc1: all warnings being treated as errors
> make[1]: *** [src/bluetoothd-adapter.o] Error 1
> ---
>  src/adapter.c |    7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)

Applied. Thanks.

Johan