[PATCH] Fix handlig for gboolean mce_bt_set field

[PATCH] Fix handlig for gboolean mce_bt_set field

[Rafal Michalski]

Expression (sigvalue & MCE_RADIO_STATE_BLUETOOTH) is invalid because it makes
wrong assumption about that it evaluates to TRUE, when bit
MCE_RADIO_STATE_BLUETOOTH is set. In this case, assignment
mce_bt_set = (sigvalue & MCE_RADIO_STATE_BLUETOOTH) makes that gboolean
"mce_bt_set" receives value equal 8 instead of TRUE (equal 1). So condition in
"if (mce_bt_set == powered)" statement will never evaluate to true because
gboolean "powered" receives only TRUE (equal 1) or FALSE (equal 0) value.
We should remember that gboolean type is not bool type in C++ sense.
It's simply typedef of gint (int), so assignmnet:
mce_bt_set = (sigvalue & MCE_RADIO_STATE_BLUETOOTH) is valid for compiler but
in our case it makes a bug. So assignmnet:
mce_bt_set = (sigvalue & MCE_RADIO_STATE_BLUETOOTH) == MCE_RADIO_STATE_BLUETOOTH
preserves that "mce_bt_set" will receive only TRUE or FALSE value.
---
 plugins/maemo6.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/plugins/maemo6.c b/plugins/maemo6.c
index c96731d..45d96b8 100644
--- a/plugins/maemo6.c
+++ b/plugins/maemo6.c
@@ -71,7 +71,8 @@ static gboolean mce_signal_callback(DBusConnection *connection,
 
 		/* set the adapter according to the mce signal
 		   and remember the value */
-		mce_bt_set = sigvalue & MCE_RADIO_STATE_BLUETOOTH;
+		mce_bt_set = (sigvalue & MCE_RADIO_STATE_BLUETOOTH) ==
+						MCE_RADIO_STATE_BLUETOOTH;
 
 		if (mce_bt_set)
 			btd_adapter_switch_online(adapter);
-- 
1.6.3.3

Re: [PATCH] Fix handlig for gboolean mce_bt_set field

[Gustavo F. Padovan]

Hi Rafal,

* Rafal Michalski <michalski.raf@gmail.com> [2010-09-30 10:45:34 +0200]:

> Expression (sigvalue & MCE_RADIO_STATE_BLUETOOTH) is invalid because it makes
> wrong assumption about that it evaluates to TRUE, when bit
> MCE_RADIO_STATE_BLUETOOTH is set. In this case, assignment
> mce_bt_set = (sigvalue & MCE_RADIO_STATE_BLUETOOTH) makes that gboolean
> "mce_bt_set" receives value equal 8 instead of TRUE (equal 1). So condition in
> "if (mce_bt_set == powered)" statement will never evaluate to true because
> gboolean "powered" receives only TRUE (equal 1) or FALSE (equal 0) value.
> We should remember that gboolean type is not bool type in C++ sense.
> It's simply typedef of gint (int), so assignmnet:
> mce_bt_set = (sigvalue & MCE_RADIO_STATE_BLUETOOTH) is valid for compiler but
> in our case it makes a bug. So assignmnet:
> mce_bt_set = (sigvalue & MCE_RADIO_STATE_BLUETOOTH) == MCE_RADIO_STATE_BLUETOOTH
> preserves that "mce_bt_set" will receive only TRUE or FALSE value.
> ---
>  plugins/maemo6.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/plugins/maemo6.c b/plugins/maemo6.c
> index c96731d..45d96b8 100644
> --- a/plugins/maemo6.c
> +++ b/plugins/maemo6.c
> @@ -71,7 +71,8 @@ static gboolean mce_signal_callback(DBusConnection *connection,
>  
>  		/* set the adapter according to the mce signal
>  		   and remember the value */
> -		mce_bt_set = sigvalue & MCE_RADIO_STATE_BLUETOOTH;
> +		mce_bt_set = (sigvalue & MCE_RADIO_STATE_BLUETOOTH) ==
> +						MCE_RADIO_STATE_BLUETOOTH;

Alternatively you can do 
	mce_bt_set = !!(sigvalue & MCE_RADIO_STATE_BLUETOOTH)

It might be better do that this way, but I'm not really asking you to
change that. ;)

-- 
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

Re: [PATCH] Fix handlig for gboolean mce_bt_set field

[Anderson Lizardo]

On Thu, Sep 30, 2010 at 4:51 AM, Gustavo F. Padovan
<padovan@profusion.mobi> wrote:
>> diff --git a/plugins/maemo6.c b/plugins/maemo6.c
>> index c96731d..45d96b8 100644
>> --- a/plugins/maemo6.c
>> +++ b/plugins/maemo6.c
>> @@ -71,7 +71,8 @@ static gboolean mce_signal_callback(DBusConnection *connection,
>>
>>               /* set the adapter according to the mce signal
>>                  and remember the value */
>> -             mce_bt_set = sigvalue & MCE_RADIO_STATE_BLUETOOTH;
>> +             mce_bt_set = (sigvalue & MCE_RADIO_STATE_BLUETOOTH) ==
>> +                                             MCE_RADIO_STATE_BLUETOOTH;
>
> Alternatively you can do
>        mce_bt_set = !!(sigvalue & MCE_RADIO_STATE_BLUETOOTH)

I'm a bit late, but:

mce_bt_set = (sigvalue & MCE_RADIO_STATE_BLUETOOTH) != 0;

would equally work.

PS: yes, I know it's already commited. :). Interestingly, there is
only one more occurrence of this idiom already in bluez code:

input/device.c:	connected = !!g_slist_find_custom(idev->connections, NULL,

Regards,
-- 
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil

[PATCH] Fix handlig for gboolean mce_bt_set field

[Rafal Michalski]

Expression (sigvalue & MCE_RADIO_STATE_BLUETOOTH) is invalid because it makes
wrong assumption about that it evaluates to TRUE, when bit
MCE_RADIO_STATE_BLUETOOTH is set. In this case, assignment
mce_bt_set = (sigvalue & MCE_RADIO_STATE_BLUETOOTH) makes that gboolean
"mce_bt_set" receives value equal 8 instead of TRUE (equal 1). So condition in
"if (mce_bt_set == powered)" statement will never evaluate to true because
gboolean "powered" receives only TRUE (equal 1) or FALSE (equal 0) value.
We should remember that gboolean type is not bool type in C++ sense.
It's simply typedef of gint (int), so assignmnet:
mce_bt_set = (sigvalue & MCE_RADIO_STATE_BLUETOOTH) is valid for compiler but
in our case it makes a bug. So normalization (double negation) in assignment:
mce_bt_set = !!(sigvalue & MCE_RADIO_STATE_BLUETOOTH) preserves that
"mce_bt_set" will receive only TRUE or FALSE value.
---
 plugins/maemo6.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/plugins/maemo6.c b/plugins/maemo6.c
index c96731d..c396db0 100644
--- a/plugins/maemo6.c
+++ b/plugins/maemo6.c
@@ -71,7 +71,7 @@ static gboolean mce_signal_callback(DBusConnection *connection,
 
 		/* set the adapter according to the mce signal
 		   and remember the value */
-		mce_bt_set = sigvalue & MCE_RADIO_STATE_BLUETOOTH;
+		mce_bt_set = !!(sigvalue & MCE_RADIO_STATE_BLUETOOTH);
 
 		if (mce_bt_set)
 			btd_adapter_switch_online(adapter);
-- 
1.6.3.3

Re: [PATCH] Fix handlig for gboolean mce_bt_set field

[Johan Hedberg]

Hi Rafal,

On Thu, Sep 30, 2010, Rafal Michalski wrote:
> Expression (sigvalue & MCE_RADIO_STATE_BLUETOOTH) is invalid because it makes
> wrong assumption about that it evaluates to TRUE, when bit
> MCE_RADIO_STATE_BLUETOOTH is set. In this case, assignment
> mce_bt_set = (sigvalue & MCE_RADIO_STATE_BLUETOOTH) makes that gboolean
> "mce_bt_set" receives value equal 8 instead of TRUE (equal 1). So condition in
> "if (mce_bt_set == powered)" statement will never evaluate to true because
> gboolean "powered" receives only TRUE (equal 1) or FALSE (equal 0) value.
> We should remember that gboolean type is not bool type in C++ sense.
> It's simply typedef of gint (int), so assignmnet:
> mce_bt_set = (sigvalue & MCE_RADIO_STATE_BLUETOOTH) is valid for compiler but
> in our case it makes a bug. So normalization (double negation) in assignment:
> mce_bt_set = !!(sigvalue & MCE_RADIO_STATE_BLUETOOTH) preserves that
> "mce_bt_set" will receive only TRUE or FALSE value.
> ---
>  plugins/maemo6.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Thanks. The patch has been pushed upstream with a much simplified commit
message (the above is really quite complicated to read compared to the
simplicity of the actual fix).

Johan