* [PATCH] Emit missing signal when data channel is reconnected.
@ 2011-03-25 13:19 Santiago Carot-Nemesio
2011-03-25 13:32 ` Johan Hedberg
0 siblings, 1 reply; 5+ messages in thread
From: Santiago Carot-Nemesio @ 2011-03-25 13:19 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Santiago Carot-Nemesio
Reconnections of data channels should be indicated to others
applications by using the appropriate signal.
---
health/hdp.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/health/hdp.c b/health/hdp.c
index 3c2dce1..b06fe17 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -510,14 +510,23 @@ static void hdp_mdl_reconn_cb(struct mcap_mdl *mdl, GError *err, gpointer data)
}
fd = mcap_mdl_get_fd(dc_data->hdp_chann->mdl);
- if (fd < 0)
+ if (fd < 0) {
reply = g_dbus_create_error(dc_data->msg,
ERROR_INTERFACE ".HealthError",
"Cannot get file descriptor");
+ g_dbus_send_message(dc_data->conn, reply);
+ return;
+ }
reply = g_dbus_create_reply(dc_data->msg, DBUS_TYPE_UNIX_FD, &fd,
DBUS_TYPE_INVALID);
g_dbus_send_message(dc_data->conn, reply);
+
+ g_dbus_emit_signal(dc_data->conn,
+ device_get_path(dc_data->hdp_chann->dev->dev),
+ HEALTH_DEVICE, "ChannelConnected",
+ DBUS_TYPE_OBJECT_PATH,
+ &dc_data->hdp_chann->path, DBUS_TYPE_INVALID);
}
static void hdp_get_dcpsm_cb(uint16_t dcpsm, gpointer user_data, GError *err)
--
1.7.4.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] Emit missing signal when data channel is reconnected.
2011-03-25 13:19 [PATCH] Emit missing signal when data channel is reconnected Santiago Carot-Nemesio
@ 2011-03-25 13:32 ` Johan Hedberg
2011-03-25 13:47 ` Santiago Carot
0 siblings, 1 reply; 5+ messages in thread
From: Johan Hedberg @ 2011-03-25 13:32 UTC (permalink / raw)
To: Santiago Carot-Nemesio; +Cc: linux-bluetooth
Hi,
On Fri, Mar 25, 2011, Santiago Carot-Nemesio wrote:
> Reconnections of data channels should be indicated to others
> applications by using the appropriate signal.
> ---
> health/hdp.c | 11 ++++++++++-
> 1 files changed, 10 insertions(+), 1 deletions(-)
>
> diff --git a/health/hdp.c b/health/hdp.c
> index 3c2dce1..b06fe17 100644
> --- a/health/hdp.c
> +++ b/health/hdp.c
> @@ -510,14 +510,23 @@ static void hdp_mdl_reconn_cb(struct mcap_mdl *mdl, GError *err, gpointer data)
> }
>
> fd = mcap_mdl_get_fd(dc_data->hdp_chann->mdl);
> - if (fd < 0)
> + if (fd < 0) {
> reply = g_dbus_create_error(dc_data->msg,
> ERROR_INTERFACE ".HealthError",
> "Cannot get file descriptor");
> + g_dbus_send_message(dc_data->conn, reply);
> + return;
> + }
>
> reply = g_dbus_create_reply(dc_data->msg, DBUS_TYPE_UNIX_FD, &fd,
> DBUS_TYPE_INVALID);
> g_dbus_send_message(dc_data->conn, reply);
> +
> + g_dbus_emit_signal(dc_data->conn,
> + device_get_path(dc_data->hdp_chann->dev->dev),
> + HEALTH_DEVICE, "ChannelConnected",
> + DBUS_TYPE_OBJECT_PATH,
> + &dc_data->hdp_chann->path, DBUS_TYPE_INVALID);
> }
>
> static void hdp_get_dcpsm_cb(uint16_t dcpsm, gpointer user_data, GError *err)
Looks like this patch is doing two things:
1. Fix the memory leak/missing g_dbus_send_message call for the return
value from g_dbus_create_error.
2. Add sending of the "ChannelConnected" signal.
Could you please split it into two separate patches?
Johan
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Emit missing signal when data channel is reconnected.
2011-03-25 13:32 ` Johan Hedberg
@ 2011-03-25 13:47 ` Santiago Carot
2011-03-25 16:15 ` Johan Hedberg
0 siblings, 1 reply; 5+ messages in thread
From: Santiago Carot @ 2011-03-25 13:47 UTC (permalink / raw)
To: Santiago Carot-Nemesio, linux-bluetooth; +Cc: Johan Hedberg
Hi Johan,
See coments below.
2011/3/25 Johan Hedberg <johan.hedberg@gmail.com>:
> Hi,
>
> On Fri, Mar 25, 2011, Santiago Carot-Nemesio wrote:
>> Reconnections of data channels should be indicated to others
>> applications by using the appropriate signal.
>> ---
>> health/hdp.c | 11 ++++++++++-
>> 1 files changed, 10 insertions(+), 1 deletions(-)
>>
>> diff --git a/health/hdp.c b/health/hdp.c
>> index 3c2dce1..b06fe17 100644
>> --- a/health/hdp.c
>> +++ b/health/hdp.c
>> @@ -510,14 +510,23 @@ static void hdp_mdl_reconn_cb(struct mcap_mdl *mdl, GError *err, gpointer data)
>> }
>>
>> fd = mcap_mdl_get_fd(dc_data->hdp_chann->mdl);
>> - if (fd < 0)
>> + if (fd < 0) {
>> reply = g_dbus_create_error(dc_data->msg,
>> ERROR_INTERFACE ".HealthError",
>> "Cannot get file descriptor");
>> + g_dbus_send_message(dc_data->conn, reply);
>> + return;
>> + }
>>
This is not a memory leak fix, if fd is less than 0, then reconnection
was not succesfully and then we reply with an error response to the
application but we dont not emit the CahnnelConnected signal. (return
statement).
>> reply = g_dbus_create_reply(dc_data->msg, DBUS_TYPE_UNIX_FD, &fd,
>> DBUS_TYPE_INVALID);
>> g_dbus_send_message(dc_data->conn, reply);
At this point we have a valid file descriptor and we can send the signal.
>> +
>> + g_dbus_emit_signal(dc_data->conn,
>> + device_get_path(dc_data->hdp_chann->dev->dev),
>> + HEALTH_DEVICE, "ChannelConnected",
>> + DBUS_TYPE_OBJECT_PATH,
>> + &dc_data->hdp_chann->path, DBUS_TYPE_INVALID);
>> }
>>
>> static void hdp_get_dcpsm_cb(uint16_t dcpsm, gpointer user_data, GError *err)
>
> Looks like this patch is doing two things:
>
> 1. Fix the memory leak/missing g_dbus_send_message call for the return
> value from g_dbus_create_error.
>
> 2. Add sending of the "ChannelConnected" signal.
>
> Could you please split it into two separate patches?
>
> Johan
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Emit missing signal when data channel is reconnected.
2011-03-25 13:47 ` Santiago Carot
@ 2011-03-25 16:15 ` Johan Hedberg
2011-03-25 16:46 ` Santiago Carot
0 siblings, 1 reply; 5+ messages in thread
From: Johan Hedberg @ 2011-03-25 16:15 UTC (permalink / raw)
To: Santiago Carot; +Cc: linux-bluetooth
Hi,
On Fri, Mar 25, 2011, Santiago Carot wrote:
> >> diff --git a/health/hdp.c b/health/hdp.c
> >> index 3c2dce1..b06fe17 100644
> >> --- a/health/hdp.c
> >> +++ b/health/hdp.c
> >> @@ -510,14 +510,23 @@ static void hdp_mdl_reconn_cb(struct mcap_mdl *mdl, GError *err, gpointer data)
> >> }
> >>
> >> fd = mcap_mdl_get_fd(dc_data->hdp_chann->mdl);
> >> - if (fd < 0)
> >> + if (fd < 0) {
> >> reply = g_dbus_create_error(dc_data->msg,
> >> ERROR_INTERFACE ".HealthError",
> >> "Cannot get file descriptor");
> >> + g_dbus_send_message(dc_data->conn, reply);
> >> + return;
> >> + }
> >>
>
> This is not a memory leak fix, if fd is less than 0, then reconnection
> was not succesfully and then we reply with an error response to the
> application but we dont not emit the CahnnelConnected signal. (return
> statement).
The code (before your patch) was assigning a value to reply and then
right afterwards assigning another value to it without freeing it in
between:
> >> reply = g_dbus_create_reply(dc_data->msg, DBUS_TYPE_UNIX_FD, &fd,
> >> DBUS_TYPE_INVALID);
How is that not a memory leak? (not to mention that g_dbus_send_message
isn't called to the error message that you create). Looks to me like
there was an "else" statement missing or something similar.
Johan
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Emit missing signal when data channel is reconnected.
2011-03-25 16:15 ` Johan Hedberg
@ 2011-03-25 16:46 ` Santiago Carot
0 siblings, 0 replies; 5+ messages in thread
From: Santiago Carot @ 2011-03-25 16:46 UTC (permalink / raw)
To: Santiago Carot, linux-bluetooth; +Cc: Johan Hedberg
Hi,
2011/3/25 Johan Hedberg <johan.hedberg@gmail.com>:
> Hi,
>
> On Fri, Mar 25, 2011, Santiago Carot wrote:
>> >> diff --git a/health/hdp.c b/health/hdp.c
>> >> index 3c2dce1..b06fe17 100644
>> >> --- a/health/hdp.c
>> >> +++ b/health/hdp.c
>> >> @@ -510,14 +510,23 @@ static void hdp_mdl_reconn_cb(struct mcap_mdl *mdl, GError *err, gpointer data)
>> >> }
>> >>
>> >> fd = mcap_mdl_get_fd(dc_data->hdp_chann->mdl);
>> >> - if (fd < 0)
>> >> + if (fd < 0) {
>> >> reply = g_dbus_create_error(dc_data->msg,
>> >> ERROR_INTERFACE ".HealthError",
>> >> "Cannot get file descriptor");
>> >> + g_dbus_send_message(dc_data->conn, reply);
>> >> + return;
>> >> + }
>> >>
>>
>> This is not a memory leak fix, if fd is less than 0, then reconnection
>> was not succesfully and then we reply with an error response to the
>> application but we dont not emit the CahnnelConnected signal. (return
>> statement).
>
> The code (before your patch) was assigning a value to reply and then
> right afterwards assigning another value to it without freeing it in
> between:
>
>> >> reply = g_dbus_create_reply(dc_data->msg, DBUS_TYPE_UNIX_FD, &fd,
>> >> DBUS_TYPE_INVALID);
>
> How is that not a memory leak? (not to mention that g_dbus_send_message
> isn't called to the error message that you create). Looks to me like
> there was an "else" statement missing or something similar.
>
Sorry,
I didn't understood what you meant. I though that you was referring to
the pach but you was referring to the old code. Now I see.
You are right, I'm going to split it in two patches right away.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-03-25 16:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-25 13:19 [PATCH] Emit missing signal when data channel is reconnected Santiago Carot-Nemesio
2011-03-25 13:32 ` Johan Hedberg
2011-03-25 13:47 ` Santiago Carot
2011-03-25 16:15 ` Johan Hedberg
2011-03-25 16:46 ` Santiago Carot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox