[PATCH BlueZ v0] gdbus: Fix compilation error if macro "error" is defined

[PATCH BlueZ v0] gdbus: Fix compilation error if macro "error" is defined

[Jaganath Kanakkassery]

The variable "signature" used in error is not defined.
---
v0 -> Made error message as per Lucas's suggestion

 gdbus/object.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdbus/object.c b/gdbus/object.c
index 900e7ab..9689006 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -645,8 +645,8 @@ static dbus_bool_t emit_signal_valist(DBusConnection *conn,
 		goto fail;
 
 	if (g_dbus_args_have_signature(args, signal) == FALSE) {
-		error("%s.%s: expected signature'%s' but got '%s'",
-				interface, name, args, signature);
+		error("%s.%s: got unexpected signature '%s'", interface, name,
+					dbus_message_get_signature(signal));
 		ret = FALSE;
 		goto fail;
 	}
-- 
1.7.1

Re: [PATCH BlueZ v0] gdbus: Fix compilation error if macro "error" is defined

[Lucas De Marchi]

On Wed, Aug 1, 2012 at 7:26 AM, Jaganath Kanakkassery
<jaganath.k@samsung.com> wrote:
> The variable "signature" used in error is not defined.
> ---
> v0 -> Made error message as per Lucas's suggestion
>
>  gdbus/object.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gdbus/object.c b/gdbus/object.c
> index 900e7ab..9689006 100644
> --- a/gdbus/object.c
> +++ b/gdbus/object.c
> @@ -645,8 +645,8 @@ static dbus_bool_t emit_signal_valist(DBusConnection *conn,
>                 goto fail;
>
>         if (g_dbus_args_have_signature(args, signal) == FALSE) {
> -               error("%s.%s: expected signature'%s' but got '%s'",
> -                               interface, name, args, signature);
> +               error("%s.%s: got unexpected signature '%s'", interface, name,
> +                                       dbus_message_get_signature(signal));
>                 ret = FALSE;
>                 goto fail;

Ack.

Lucas De Marchi

Re: [PATCH BlueZ v0] gdbus: Fix compilation error if macro "error" is defined

[Jaganath Kanakkassery]

Hi,

--------------------------------------------------
From: "Lucas De Marchi" <lucas.demarchi@profusion.mobi>
Sent: Monday, August 13, 2012 7:33 PM
To: "Jaganath Kanakkassery" <jaganath.k@samsung.com>
Cc: <linux-bluetooth@vger.kernel.org>
Subject: Re: [PATCH BlueZ v0] gdbus: Fix compilation error if macro "error" 
is defined

> On Wed, Aug 1, 2012 at 7:26 AM, Jaganath Kanakkassery
> <jaganath.k@samsung.com> wrote:
>> The variable "signature" used in error is not defined.
>> ---
>> v0 -> Made error message as per Lucas's suggestion
>>
>>  gdbus/object.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/gdbus/object.c b/gdbus/object.c
>> index 900e7ab..9689006 100644
>> --- a/gdbus/object.c
>> +++ b/gdbus/object.c
>> @@ -645,8 +645,8 @@ static dbus_bool_t emit_signal_valist(DBusConnection 
>> *conn,
>>                 goto fail;
>>
>>         if (g_dbus_args_have_signature(args, signal) == FALSE) {
>> -               error("%s.%s: expected signature'%s' but got '%s'",
>> -                               interface, name, args, signature);
>> +               error("%s.%s: got unexpected signature '%s'", interface, 
>> name,
>> + 
>> dbus_message_get_signature(signal));
>>                 ret = FALSE;
>>                 goto fail;
>
> Ack.
>
> Lucas De Marchi
> --

Ping.

Thanks,
Jaganath 

Re: [PATCH BlueZ v0] gdbus: Fix compilation error if macro "error" is defined

[Johan Hedberg]

Hi Jaganath,

On Wed, Aug 01, 2012, Jaganath Kanakkassery wrote:
> The variable "signature" used in error is not defined.
> ---
> v0 -> Made error message as per Lucas's suggestion
> 
>  gdbus/object.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/gdbus/object.c b/gdbus/object.c
> index 900e7ab..9689006 100644
> --- a/gdbus/object.c
> +++ b/gdbus/object.c
> @@ -645,8 +645,8 @@ static dbus_bool_t emit_signal_valist(DBusConnection *conn,
>  		goto fail;
>  
>  	if (g_dbus_args_have_signature(args, signal) == FALSE) {
> -		error("%s.%s: expected signature'%s' but got '%s'",
> -				interface, name, args, signature);
> +		error("%s.%s: got unexpected signature '%s'", interface, name,
> +					dbus_message_get_signature(signal));
>  		ret = FALSE;
>  		goto fail;
>  	}

What's the reason for dropping printing of the value of "args"? I'd
think that's quite useful in figuring out what went wrong. Even if there
is a reason to drop it it has nothing to do with fixing the compilation
error due to undefined "signature" and should therefore be in its own
patch.

Johan

Re: [PATCH BlueZ v0] gdbus: Fix compilation error if macro "error" is defined

[Jaganath Kanakkassery]

Hi Johan,

--------------------------------------------------
From: "Johan Hedberg" <johan.hedberg@gmail.com>
Sent: Thursday, August 16, 2012 1:19 PM
To: "Jaganath Kanakkassery" <jaganath.k@samsung.com>
Cc: <linux-bluetooth@vger.kernel.org>
Subject: Re: [PATCH BlueZ v0] gdbus: Fix compilation error if macro "error" 
is defined

> Hi Jaganath,
>
> On Wed, Aug 01, 2012, Jaganath Kanakkassery wrote:
>> The variable "signature" used in error is not defined.
>> ---
>> v0 -> Made error message as per Lucas's suggestion
>>
>>  gdbus/object.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/gdbus/object.c b/gdbus/object.c
>> index 900e7ab..9689006 100644
>> --- a/gdbus/object.c
>> +++ b/gdbus/object.c
>> @@ -645,8 +645,8 @@ static dbus_bool_t emit_signal_valist(DBusConnection 
>> *conn,
>>  goto fail;
>>
>>  if (g_dbus_args_have_signature(args, signal) == FALSE) {
>> - error("%s.%s: expected signature'%s' but got '%s'",
>> - interface, name, args, signature);
>> + error("%s.%s: got unexpected signature '%s'", interface, name,
>> + dbus_message_get_signature(signal));
>>  ret = FALSE;
>>  goto fail;
>>  }
>
> What's the reason for dropping printing of the value of "args"? I'd
> think that's quite useful in figuring out what went wrong. Even if there
> is a reason to drop it it has nothing to do with fixing the compilation
> error due to undefined "signature" and should therefore be in its own
> patch.
>

"args" is a structure now and that's the reason it is removed and I think
the complete signature of args cannot be available at the point where it is 
printed.
How about changing the commit message to "Correct error message"?

Thanks,
Jaganath 

Re: [PATCH BlueZ v0] gdbus: Fix compilation error if macro "error" is defined

[Johan Hedberg]

Hi Jaganath,

On Thu, Aug 16, 2012, Jaganath Kanakkassery wrote:
> >On Wed, Aug 01, 2012, Jaganath Kanakkassery wrote:
> >>The variable "signature" used in error is not defined.
> >>---
> >>v0 -> Made error message as per Lucas's suggestion
> >>
> >> gdbus/object.c |    4 ++--
> >> 1 files changed, 2 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/gdbus/object.c b/gdbus/object.c
> >>index 900e7ab..9689006 100644
> >>--- a/gdbus/object.c
> >>+++ b/gdbus/object.c
> >>@@ -645,8 +645,8 @@ static dbus_bool_t
> >>emit_signal_valist(DBusConnection *conn,
> >> goto fail;
> >>
> >> if (g_dbus_args_have_signature(args, signal) == FALSE) {
> >>- error("%s.%s: expected signature'%s' but got '%s'",
> >>- interface, name, args, signature);
> >>+ error("%s.%s: got unexpected signature '%s'", interface, name,
> >>+ dbus_message_get_signature(signal));
> >> ret = FALSE;
> >> goto fail;
> >> }
> >
> >What's the reason for dropping printing of the value of "args"? I'd
> >think that's quite useful in figuring out what went wrong. Even if there
> >is a reason to drop it it has nothing to do with fixing the compilation
> >error due to undefined "signature" and should therefore be in its own
> >patch.
> >
> 
> "args" is a structure now and that's the reason it is removed and I think
> the complete signature of args cannot be available at the point
> where it is printed.
> How about changing the commit message to "Correct error message"?

No, keep the summary line as is, but please add a note about this args
variable to the rest of the message, i.e. something like "The variable
"signature" is not defined and "args" is now a struct instead of a
string".

Johan

Re: [PATCH BlueZ v0] gdbus: Fix compilation error if macro "error" is defined

[Jaganath Kanakkassery]

Hi Johan,

--------------------------------------------------
From: "Johan Hedberg" <johan.hedberg@gmail.com>
Sent: Thursday, August 16, 2012 6:00 PM
To: "Jaganath Kanakkassery" <jaganath.k@samsung.com>
Cc: <linux-bluetooth@vger.kernel.org>
Subject: Re: [PATCH BlueZ v0] gdbus: Fix compilation error if macro "error" 
is defined

> Hi Jaganath,
>
> On Thu, Aug 16, 2012, Jaganath Kanakkassery wrote:
>> >On Wed, Aug 01, 2012, Jaganath Kanakkassery wrote:
>> >>The variable "signature" used in error is not defined.
>> >>---
>> >>v0 -> Made error message as per Lucas's suggestion
>> >>
>> >> gdbus/object.c |    4 ++--
>> >> 1 files changed, 2 insertions(+), 2 deletions(-)
>> >>
>> >>diff --git a/gdbus/object.c b/gdbus/object.c
>> >>index 900e7ab..9689006 100644
>> >>--- a/gdbus/object.c
>> >>+++ b/gdbus/object.c
>> >>@@ -645,8 +645,8 @@ static dbus_bool_t
>> >>emit_signal_valist(DBusConnection *conn,
>> >> goto fail;
>> >>
>> >> if (g_dbus_args_have_signature(args, signal) == FALSE) {
>> >>- error("%s.%s: expected signature'%s' but got '%s'",
>> >>- interface, name, args, signature);
>> >>+ error("%s.%s: got unexpected signature '%s'", interface, name,
>> >>+ dbus_message_get_signature(signal));
>> >> ret = FALSE;
>> >> goto fail;
>> >> }
>> >
>> >What's the reason for dropping printing of the value of "args"? I'd
>> >think that's quite useful in figuring out what went wrong. Even if there
>> >is a reason to drop it it has nothing to do with fixing the compilation
>> >error due to undefined "signature" and should therefore be in its own
>> >patch.
>> >
>>
>> "args" is a structure now and that's the reason it is removed and I think
>> the complete signature of args cannot be available at the point
>> where it is printed.
>> How about changing the commit message to "Correct error message"?
>
> No, keep the summary line as is, but please add a note about this args
> variable to the rest of the message, i.e. something like "The variable
> "signature" is not defined and "args" is now a struct instead of a
> string".

Ok, I will raise v1,

Thanks,
Jaganath