Hi Johan,
On Thu, Jul 14, 2011 at 2:12 PM, Johan Hedberg <johan.hedberg@xxxxxxxxx> wrote:
> Hi Claudio,
>
> On Wed, Jul 06, 2011, Claudio Takahasi wrote:
>> Connection should not be dropped if there is at least one ATT connection
>> callback registered.
>> ---
>> src/device.c | 19 ++++++++++++-------
>> 1 files changed, 12 insertions(+), 7 deletions(-)
>
> Patches 8 and 9 have now been pushed, but there's some reference
> counting weirdness going on here again:
>
>> --- a/src/device.c
>> +++ b/src/device.c
>> @@ -1664,6 +1664,7 @@ static void primary_cb(GSList *services, guint8 status, gpointer user_data)
>> done:
>> device->browse = NULL;
>> browse_request_free(req, shutdown);
>> + g_attrib_unref(device->attrib);
>> }
>
> Where's the device->attrib = NULL?
>
>> @@ -2568,18 +2569,22 @@ guint btd_device_add_attio_callback(struct btd_device *device,
>> attio->dcfunc = dcfunc;
>> attio->user_data = user_data;
>>
>> - device->attios = g_slist_append(device->attios, attio);
>> -
>> - if (device->attrib && cfunc)
>> - cfunc(device->attrib, user_data);
>> + if (device->attrib) {
>> + /* First element */
>> + if (device->attios == NULL)
>> + device->attrib = g_attrib_ref(device->attrib);
>
> Eh? It looks like attio is supposed to own this reference created by
> g_attrib_ref() so you should really be assigning the return value to
> attio->attrib. Or is it so that device->attrib is actually there only
> for any elements in the device->attios list? If that's the case then
> it's fine to have just create device->attrib when you add the first
> element to the list and unref + set to NULL when you remove the last
> element from the list.
>
> Johan
>
Create attio->attrib is not a good approach, GAttrib is created after
the connection establishment and attio callbacks can be registered
anytime.
The patch that I will send will fix the reference counting using
device->attrib to hold the reference for attios. A new GAttrib pointer
will added in the "struct browse_req" and it will be used in the
discovery primary services. device->attrib reference will be
"released" if device->attios list is empty.
I realized that "shutdown" parameter used in browse_request_free
function can be removed if we use the list to control disconnections.
I gonna send this cleanup patch after you apply this patch series.
Regards,
Claudio.
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
