* [PATCH BlueZ] input: Fix closing sockets when ioctl_connadd() fails
From: Anderson Lizardo @ 2013-01-03 2:08 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
Instead of calling close() directly, properly shutdown the channel and
set GIOChannel pointers to NULL.
Also remove dead code and do a small refactoring around the code.
Fixes this error detected when HIDP support is disabled on kernel and we
attempt to connect to a BT keyboard:
bluetoothd[5168]: profiles/input/device.c:encrypt_notify()
bluetoothd[5168]: ioctl_connadd(): Protocol not supported(93)
bluetoothd[5168]: profiles/input/device.c:ctrl_watch_cb() Device
CA:FE:CA:FE:CA:FE disconnected
(bluetoothd:5168): GLib-WARNING **: Invalid file descriptor.
bluetoothd[5168]: profiles/input/device.c:intr_watch_cb() Device
CA:FE:CA:FE:CA:FE disconnected
(bluetoothd:5168): GLib-WARNING **: Invalid file descriptor.
---
Let me know if you want me to split the refactoring part in a separate patch.
profiles/input/device.c | 62 ++++++++++++++++++++++-------------------------
1 file changed, 29 insertions(+), 33 deletions(-)
diff --git a/profiles/input/device.c b/profiles/input/device.c
index 9cea028..2cb44cf 100644
--- a/profiles/input/device.c
+++ b/profiles/input/device.c
@@ -136,8 +136,10 @@ static gboolean intr_watch_cb(GIOChannel *chan, GIOCondition cond, gpointer data
idev->intr_watch = 0;
- g_io_channel_unref(idev->intr_io);
- idev->intr_io = NULL;
+ if (idev->intr_io) {
+ g_io_channel_unref(idev->intr_io);
+ idev->intr_io = NULL;
+ }
/* Close control channel */
if (idev->ctrl_io && !(cond & G_IO_NVAL))
@@ -163,8 +165,10 @@ static gboolean ctrl_watch_cb(GIOChannel *chan, GIOCondition cond, gpointer data
idev->ctrl_watch = 0;
- g_io_channel_unref(idev->ctrl_io);
- idev->ctrl_io = NULL;
+ if (idev->ctrl_io) {
+ g_io_channel_unref(idev->ctrl_io);
+ idev->ctrl_io = NULL;
+ }
/* Close interrupt channel */
if (idev->intr_io && !(cond & G_IO_NVAL))
@@ -271,43 +275,35 @@ static int ioctl_connadd(struct hidp_connadd_req *req)
return err;
}
-static void encrypt_completed(uint8_t status, gpointer user_data)
-{
- struct hidp_connadd_req *req = user_data;
- int err;
-
- if (status) {
- error("Encryption failed: %s(0x%x)",
- strerror(bt_error(status)), status);
- goto failed;
- }
-
- err = ioctl_connadd(req);
- if (err == 0)
- goto cleanup;
-
- error("ioctl_connadd(): %s(%d)", strerror(-err), -err);
-failed:
- close(req->intr_sock);
- close(req->ctrl_sock);
-
-cleanup:
- free(req->rd_data);
-
- g_free(req);
-}
-
static gboolean encrypt_notify(GIOChannel *io, GIOCondition condition,
gpointer data)
{
struct input_device *idev = data;
- struct hidp_connadd_req *req = idev->req;
+ int err;
- DBG(" ");
+ DBG("");
- encrypt_completed(0, req);
+ err = ioctl_connadd(idev->req);
+ if (err < 0) {
+ error("ioctl_connadd(): %s (%d)", strerror(-err), -err);
+
+ if (idev->ctrl_io) {
+ g_io_channel_shutdown(idev->ctrl_io, FALSE, NULL);
+ g_io_channel_unref(idev->ctrl_io);
+ idev->ctrl_io = NULL;
+ }
+
+ if (idev->intr_io) {
+ g_io_channel_shutdown(idev->intr_io, FALSE, NULL);
+ g_io_channel_unref(idev->intr_io);
+ idev->intr_io = NULL;
+ }
+ }
idev->sec_watch = 0;
+
+ g_free(idev->req->rd_data);
+ g_free(idev->req);
idev->req = NULL;
return FALSE;
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH 1/1] adapter, AVCTP: Replaced calls to g_queue_free_full function
From: Marcel Holtmann @ 2013-01-02 23:42 UTC (permalink / raw)
To: Luiz Augusto von Dentz
Cc: Vinicius Gomes, Giovanni Gherdovich, Anderson Lizardo,
linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZL=4ToEwR1i6bihLKWQTn6hurEwWpjQ0gF0rgkO7eExLQ@mail.gmail.com>
Hi Luiz,
> >> In that case I would just revert back this patch, but the
> >> documentation actually say g_slist_free_full is available since 2.28
> >> http://developer.gnome.org/glib/2.28/glib-Singly-Linked-Lists.html#g-slist-free-full
> >> so I wonder what is going on.
> >>
> >
> > The problem now is g_queue_free_full() not the g_slist_free_full().
>
> Right, but it is quite the same situation and I don't get why we don't
> just update, by the time distros start to package BlueZ 5 glib 2.32
> wont be a problem, in fact it should not be a problem right now as it
> is about a year old release:
because every new GLib release drags in more dependencies. It is a bit
out of control. So requiring the 2.32 comes at a cost that I am not
willing to pay right now. We already have seen this with ConnMan where I
accidentally used a newer GLib function that was not present in a 2.28
and before. It is pretty hard for embedded system to do these kind of
upgrades when their dependencies and thus footprint and memory
consumption increases for just a simple convenience function.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 2/2] AVCTP: Replace calls to g_queue_free_full function
From: Marcel Holtmann @ 2013-01-02 22:10 UTC (permalink / raw)
To: Giovanni Gherdovich; +Cc: linux-bluetooth
In-Reply-To: <1357162350-14253-1-git-send-email-g.gherdovich@gmail.com>
Hi Giovanni,
> The function g_queue_free_full is available only from GLib 2.32.
> If BlueZ has to build against GLib 2.28, as stated in the configure.ac,
> this patch replaces the calls to g_queue_free_full in the AVTCP module
> with its body, taken from the sources of GLib 2.32.
> ---
> profiles/audio/avctp.c | 12 +++++++++++-
> 1 files changed, 11 insertions(+), 1 deletions(-)
>
> diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
> index 013c587..7a5a096 100644
> --- a/profiles/audio/avctp.c
> +++ b/profiles/audio/avctp.c
> @@ -383,6 +383,15 @@ static void pending_destroy(void *data)
> g_free(req);
> }
>
> +static void pending_destroy_wrapper(void *data, void *dummy)
> +{
> + /*
> + * Wrapper around pending_destroy to match the signature
> + * required for the second argument of g_queue_foreach.
> + */
> + pending_destroy(data);
> +}
> +
change pending_destroy() to this:
static void pending_destroy(gpointer data, gpointer user_data)
{
...
}
And then fix the callers to add an extra NULL.
> static void avctp_channel_destroy(struct avctp_channel *chan)
> {
> g_io_channel_shutdown(chan->io, TRUE, NULL);
> @@ -395,7 +404,8 @@ static void avctp_channel_destroy(struct avctp_channel *chan)
> g_source_remove(chan->process_id);
>
> g_free(chan->buffer);
> - g_queue_free_full(chan->queue, pending_destroy);
> + g_queue_foreach(chan->queue, pending_destroy_wrapper, NULL);
> + g_queue_free(chan->queue);
> g_slist_free_full(chan->processed, pending_destroy);
> g_slist_free_full(chan->handlers, g_free);
> g_free(chan);
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/2] core: Replace calls to g_queue_free_full function
From: Marcel Holtmann @ 2013-01-02 22:08 UTC (permalink / raw)
To: Giovanni Gherdovich; +Cc: linux-bluetooth
In-Reply-To: <1357162287-14216-1-git-send-email-g.gherdovich@gmail.com>
Hi Giovanni,
> The function g_queue_free_full is available only from GLib 2.32.
> If BlueZ has to build against GLib 2.28, as stated in the configure.ac,
> this patch replaces the calls to g_queue_free_full in the "core" BlueZ module
> with its body, taken from the sources of GLib 2.32.
> ---
> src/adapter.c | 12 +++++++++++-
> 1 files changed, 11 insertions(+), 1 deletions(-)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index e71cea8..f7fc00e 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -1688,6 +1688,15 @@ int btd_adapter_stop(struct btd_adapter *adapter)
> return 0;
> }
>
> +static void g_free_wrapper(gpointer mem, gpointer dummy)
> +{
> + /*
> + * Wrapper around GLib's g_free to match the signature
> + * required for the second argument of g_queue_foreach.
> + */
> + g_free(mem);
> +}
> +
I would have done this:
static void free_service_auth(gpointer data, gpointer user_data)
{
struct service_auth *auth = data;
g_free(auth);
}
> static void adapter_free(gpointer user_data)
> {
> struct btd_adapter *adapter = user_data;
> @@ -1697,7 +1706,8 @@ static void adapter_free(gpointer user_data)
> if (adapter->auth_idle_id)
> g_source_remove(adapter->auth_idle_id);
>
> - g_queue_free_full(adapter->auths, g_free);
> + g_queue_foreach(adapter->auths, g_free_wrapper, NULL);
> + g_queue_free(adapter->auths);
>
> sdp_list_free(adapter->services, NULL);
>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/1] adapter, AVCTP: Replaced calls to g_queue_free_full function
From: Giovanni Gherdovich @ 2013-01-02 21:43 UTC (permalink / raw)
To: Vinicius Costa Gomes, Luiz Augusto von Dentz
Cc: Anderson Lizardo, linux-bluetooth@vger.kernel.org
In-Reply-To: <20130102205437.GA2334@samus>
Hi Vinicius and Luiz,
2013/1/2 Vinicius Costa Gomes <vinicius.gomes@openbossa.org>:
> [...]
>>
>> Right, but it is quite the same situation and I don't get why we don't
>> just update, by the time distros start to package BlueZ 5 glib 2.32
>> wont be a problem, in fact it should not be a problem right now as it
>> is about a year old release:
>
> I agree with you here. And that was the suggestion that I gave to
> Giovanni on IRC when he found the problem.
I just resubmitted an amended patch, since I had already prepared it.
BTW I will run a quick check on what GLib versions the various
major distros are packaging right now, just to have some data to add
to the poll.
Regards,
Giovanni
^ permalink raw reply
* [PATCH 2/2] AVCTP: Replace calls to g_queue_free_full function
From: Giovanni Gherdovich @ 2013-01-02 21:32 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Giovanni Gherdovich
The function g_queue_free_full is available only from GLib 2.32.
If BlueZ has to build against GLib 2.28, as stated in the configure.ac,
this patch replaces the calls to g_queue_free_full in the AVTCP module
with its body, taken from the sources of GLib 2.32.
---
profiles/audio/avctp.c | 12 +++++++++++-
1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index 013c587..7a5a096 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -383,6 +383,15 @@ static void pending_destroy(void *data)
g_free(req);
}
+static void pending_destroy_wrapper(void *data, void *dummy)
+{
+ /*
+ * Wrapper around pending_destroy to match the signature
+ * required for the second argument of g_queue_foreach.
+ */
+ pending_destroy(data);
+}
+
static void avctp_channel_destroy(struct avctp_channel *chan)
{
g_io_channel_shutdown(chan->io, TRUE, NULL);
@@ -395,7 +404,8 @@ static void avctp_channel_destroy(struct avctp_channel *chan)
g_source_remove(chan->process_id);
g_free(chan->buffer);
- g_queue_free_full(chan->queue, pending_destroy);
+ g_queue_foreach(chan->queue, pending_destroy_wrapper, NULL);
+ g_queue_free(chan->queue);
g_slist_free_full(chan->processed, pending_destroy);
g_slist_free_full(chan->handlers, g_free);
g_free(chan);
--
1.7.4.1
^ permalink raw reply related
* [PATCH 1/2] core: Replace calls to g_queue_free_full function
From: Giovanni Gherdovich @ 2013-01-02 21:31 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Giovanni Gherdovich
The function g_queue_free_full is available only from GLib 2.32.
If BlueZ has to build against GLib 2.28, as stated in the configure.ac,
this patch replaces the calls to g_queue_free_full in the "core" BlueZ module
with its body, taken from the sources of GLib 2.32.
---
src/adapter.c | 12 +++++++++++-
1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index e71cea8..f7fc00e 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1688,6 +1688,15 @@ int btd_adapter_stop(struct btd_adapter *adapter)
return 0;
}
+static void g_free_wrapper(gpointer mem, gpointer dummy)
+{
+ /*
+ * Wrapper around GLib's g_free to match the signature
+ * required for the second argument of g_queue_foreach.
+ */
+ g_free(mem);
+}
+
static void adapter_free(gpointer user_data)
{
struct btd_adapter *adapter = user_data;
@@ -1697,7 +1706,8 @@ static void adapter_free(gpointer user_data)
if (adapter->auth_idle_id)
g_source_remove(adapter->auth_idle_id);
- g_queue_free_full(adapter->auths, g_free);
+ g_queue_foreach(adapter->auths, g_free_wrapper, NULL);
+ g_queue_free(adapter->auths);
sdp_list_free(adapter->services, NULL);
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH 1/1] adapter, AVCTP: Replaced calls to g_queue_free_full function
From: Vinicius Costa Gomes @ 2013-01-02 20:54 UTC (permalink / raw)
To: Luiz Augusto von Dentz
Cc: Giovanni Gherdovich, Anderson Lizardo,
linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZL=4ToEwR1i6bihLKWQTn6hurEwWpjQ0gF0rgkO7eExLQ@mail.gmail.com>
Hi Luiz,
>
> Right, but it is quite the same situation and I don't get why we don't
> just update, by the time distros start to package BlueZ 5 glib 2.32
> wont be a problem, in fact it should not be a problem right now as it
> is about a year old release:
I agree with you here. And that was the suggestion that I gave to
Giovanni on IRC when he found the problem.
>
> commit 816554c62bf227498cb539924e6ee2050030b4b9
> Author: Matthias Clasen <mclasen@redhat.com>
> Date: Sat Mar 24 11:28:35 2012 -0400
>
> 2.32.0
>
> --
> Luiz Augusto von Dentz
Cheers,
--
Vinicius
^ permalink raw reply
* [PATCH] Bluetooth device 04ca:3008 should use ath3k
From: Sergio Cambra @ 2013-01-02 20:52 UTC (permalink / raw)
To: linux-bluetooth
I'm using kernel 3.6.6 from Chakra, and 04ca:3008 device was using btusb driver. Adapter was found but scan didn't work. Loading ath3k with modprobe didn't fix it.
After adding 04ca:3008 to ath3k.c and btusb.c, ath3k was loaded on boot and it finds bluetooth devices on scanning. This patch is working in 3.6.6 but it's simple so I did against latest version (3.8-rc1)
This patch it's for bug https://bugzilla.kernel.org/show_bug.cgi?id=51891
diff -uprN -X linux-3.8-rc1-vanilla/Documentation/dontdiff linux-3.8-rc1-vanilla/drivers/bluetooth/ath3k.c linux-3.8-rc1/drivers/bluetooth/ath3k.c
--- linux-3.8-rc1-vanilla/drivers/bluetooth/ath3k.c 2013-01-02 21:41:46.778002039 +0100
+++ linux-3.8-rc1/drivers/bluetooth/ath3k.c 2013-01-02 21:41:55.501028313 +0100
@@ -77,6 +77,7 @@ static struct usb_device_id ath3k_table[
{ USB_DEVICE(0x0CF3, 0x311D) },
{ USB_DEVICE(0x13d3, 0x3375) },
{ USB_DEVICE(0x04CA, 0x3005) },
+ { USB_DEVICE(0x04CA, 0x3008) },
{ USB_DEVICE(0x13d3, 0x3362) },
{ USB_DEVICE(0x0CF3, 0xE004) },
{ USB_DEVICE(0x0930, 0x0219) },
@@ -104,6 +105,7 @@ static struct usb_device_id ath3k_blist_
{ USB_DEVICE(0x0cf3, 0x311D), .driver_info = BTUSB_ATH3012 },
{ USB_DEVICE(0x13d3, 0x3375), .driver_info = BTUSB_ATH3012 },
{ USB_DEVICE(0x04ca, 0x3005), .driver_info = BTUSB_ATH3012 },
+ { USB_DEVICE(0x04ca, 0x3008), .driver_info = BTUSB_ATH3012 },
{ USB_DEVICE(0x13d3, 0x3362), .driver_info = BTUSB_ATH3012 },
{ USB_DEVICE(0x0cf3, 0xe004), .driver_info = BTUSB_ATH3012 },
{ USB_DEVICE(0x0930, 0x0219), .driver_info = BTUSB_ATH3012 },
diff -uprN -X linux-3.8-rc1-vanilla/Documentation/dontdiff linux-3.8-rc1-vanilla/drivers/bluetooth/btusb.c linux-3.8-rc1/drivers/bluetooth/btusb.c
--- linux-3.8-rc1-vanilla/drivers/bluetooth/btusb.c 2013-01-02 21:41:46.781335256 +0100
+++ linux-3.8-rc1/drivers/bluetooth/btusb.c 2013-01-02 21:41:55.504361528 +0100
@@ -135,6 +135,7 @@ static struct usb_device_id blacklist_ta
{ USB_DEVICE(0x0cf3, 0x311d), .driver_info = BTUSB_ATH3012 },
{ USB_DEVICE(0x13d3, 0x3375), .driver_info = BTUSB_ATH3012 },
{ USB_DEVICE(0x04ca, 0x3005), .driver_info = BTUSB_ATH3012 },
+ { USB_DEVICE(0x04ca, 0x3008), .driver_info = BTUSB_ATH3012 },
{ USB_DEVICE(0x13d3, 0x3362), .driver_info = BTUSB_ATH3012 },
{ USB_DEVICE(0x0cf3, 0xe004), .driver_info = BTUSB_ATH3012 },
{ USB_DEVICE(0x0930, 0x0219), .driver_info = BTUSB_ATH3012 },
^ permalink raw reply
* Re: [PATCH 1/1] adapter, AVCTP: Replaced calls to g_queue_free_full function
From: Luiz Augusto von Dentz @ 2013-01-02 20:35 UTC (permalink / raw)
To: Vinicius Gomes
Cc: Giovanni Gherdovich, Anderson Lizardo,
linux-bluetooth@vger.kernel.org
In-Reply-To: <CAE6HaMmrc9BEW-cjmw3fkZptCzvr14M0GtKwoKXuqi8or1zeWg@mail.gmail.com>
Hi Vinicius,
On Wed, Jan 2, 2013 at 10:23 PM, Vinicius Gomes
<vinicius.gomes@openbossa.org> wrote:
> Hi Luiz,
>
>> In that case I would just revert back this patch, but the
>> documentation actually say g_slist_free_full is available since 2.28
>> http://developer.gnome.org/glib/2.28/glib-Singly-Linked-Lists.html#g-slist-free-full
>> so I wonder what is going on.
>>
>
> The problem now is g_queue_free_full() not the g_slist_free_full().
Right, but it is quite the same situation and I don't get why we don't
just update, by the time distros start to package BlueZ 5 glib 2.32
wont be a problem, in fact it should not be a problem right now as it
is about a year old release:
commit 816554c62bf227498cb539924e6ee2050030b4b9
Author: Matthias Clasen <mclasen@redhat.com>
Date: Sat Mar 24 11:28:35 2012 -0400
2.32.0
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: [PATCH 1/1] adapter, AVCTP: Replaced calls to g_queue_free_full function
From: Vinicius Gomes @ 2013-01-02 20:23 UTC (permalink / raw)
To: Luiz Augusto von Dentz
Cc: Giovanni Gherdovich, Anderson Lizardo,
linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZJxF6H7V-e=6wMYgQSM4C7FfXW9FCrdRZ=B6U1YgLQSSQ@mail.gmail.com>
Hi Luiz,
> In that case I would just revert back this patch, but the
> documentation actually say g_slist_free_full is available since 2.28
> http://developer.gnome.org/glib/2.28/glib-Singly-Linked-Lists.html#g-slist-free-full
> so I wonder what is going on.
>
The problem now is g_queue_free_full() not the g_slist_free_full().
>
>
> --
> Luiz Augusto von Dentz
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Cheers,
--
Vinicius
^ permalink raw reply
* Re: [PATCH 1/1] adapter, AVCTP: Replaced calls to g_queue_free_full function
From: Luiz Augusto von Dentz @ 2013-01-02 19:58 UTC (permalink / raw)
To: Giovanni Gherdovich; +Cc: Anderson Lizardo, linux-bluetooth@vger.kernel.org
In-Reply-To: <CAJN7YmNzo1ya34tdgyffcBf=SLt4RsJ=2QaSy3fD6meKx7wnsA@mail.gmail.com>
Hi Giovanni,
On Tue, Jan 1, 2013 at 12:19 PM, Giovanni Gherdovich
<g.gherdovich@gmail.com> wrote:
> Hi Anderson,
>
> thank you for your review.
> A few comments below.
>
> 2012/12/30 Anderson Lizardo <anderson.lizardo@openbossa.org>:
>> [...]
>>> diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
>>> index 013c587..745ced8 100644
>>> --- a/profiles/audio/avctp.c
>>> +++ b/profiles/audio/avctp.c
>>> @@ -395,7 +395,8 @@ static void avctp_channel_destroy(struct avctp_channel *chan)
>>> g_source_remove(chan->process_id);
>>>
>>> g_free(chan->buffer);
>>> - g_queue_free_full(chan->queue, pending_destroy);
>>> + g_queue_foreach(chan->queue, (GFunc)pending_destroy, NULL);
>>> + g_queue_free(chan->queue);
>>
>> Where possible, try to avoid using casts for functions. In this case,
>> try removing "(GFunc)" and see if code still compiles cleanly with
>> "./bootstrap-configure && make".
>
> You rise a very good point here.
> In this case, just removing the cast doesn't work:
> the function "g_queue_foreach" is expecting an object of type
>
> void (*) (void *, void *)
>
> as second argument, while "pending_destroy" has type
>
> void (*) (void *)
>
> "GFunc" is a typedef to "void (*) (void *, void *)", and the
> cast is required to make the code compile and run.
>
> This brings us to the central issue: the patch I submitted,
> as well as the GLib code from which I cut-and-pasted the body
> of the function g_queue_free_full, strictly speaking
> relies on "undefined behaviour", since you cast a function pointer
> to another of incompatible type.
> In this stackoverflow thread somebody offers an extract of
> the C standard where this issue is discussed:
> http://stackoverflow.com/questions/559581/casting-a-function-pointer-to-another-type
>
> I submitted a question to the GLib developers, asking them
> why they do so and how they expect their code to work:
> https://mail.gnome.org/archives/gtk-devel-list/2012-December/msg00061.html
>
> I will quote here the answer I got, since it wasn't stored in their
> mailing list archives, and the argument makes a lot of sense in my opinion:
>
> "
> In order to support varargs ('...', http://en.wikipedia.org/wiki/Stdarg.h)
> C compilers put function call arguments backwards on the stack. This allows
> functions that don't care about extra arguments to simply not offset
> back far enough
> on the stack to notice them. No modern C compiler excludes support
> for varargs and glib relies on varargs anyway.
> So its not really an issue."
>
> Which is: you can cast a unary function to a binary function type;
> the extra argument will just be ignored, even if the standard takes
> a more safe approach and says "don't do that".
>
> My understanding is that the real danger is if you do the opposite cast,
> i.e. a binary function f casted to a unary function type:
> you will then feed to f less arguments that it expects,
> and it will then corrupt the stack looking for the missing input.
>
> In order to solve this special issue in the BlueZ codebase in
> a standard-compliant way, one would have to rewrite the function
> "g_queue_foreach" so that it takes a unary function as second argument;
> but as far as I understood, GLib code relies heavily on this kind
> of "forbidden casts"; here another message from last week where a
> developer was asking a question very similar to mine,
> https://mail.gnome.org/archives/gtk-devel-list/2012-December/msg00032.html
> and the answer is basically: "you found just one of the many instances
> where we do that, and in practice it works just fine."
>
> To summarize:
> I am resubmitting my patch amended with all your requests apart from
> the casting issue. If you still have strong objections against it,
> I will re-submit again rewriting the function "g_queue_foreach"
> with the right prototype, taking it out from GLib and putting it in
> "Bluez space".
>
> Regards,
> Giovanni
> --
There is something off here, in the past we did have an implementation
of g_slist_free_full to overcome this dependency problem but it was
removed in this commit:
commit 84156dadb25ec0973752d34d84fc9ffb3c720988
Author: Marcel Holtmann <marcel@holtmann.org>
Date: Mon Apr 16 18:22:24 2012 +0200
build: Remove glib-compat.h support
In that case I would just revert back this patch, but the
documentation actually say g_slist_free_full is available since 2.28
http://developer.gnome.org/glib/2.28/glib-Singly-Linked-Lists.html#g-slist-free-full
so I wonder what is going on.
--
Luiz Augusto von Dentz
^ permalink raw reply
* FLUSHING ACL PACKETS
From: Ajay @ 2013-01-02 18:47 UTC (permalink / raw)
To: linux-bluetooth
[-- Attachment #1: Type: text/plain, Size: 455 bytes --]
Hi ,
I wanted to moniter the acl flushed packet numbers. In my L2cap
client server code , i did the following steps
1. Enabled setsocket() option as BT_FLUSHABLE
2 . set automatic flush timeout as 50msec
On data transfer, i could see the flush occured event frequently on
hcidump . so how do i get the number of packets flushed???
kindly suggest me some solution...
--
Thanks & Regards
AJAY KV
GlobalEdge software Ltd
8892753703
[-- Attachment #2: ajay_kv.vcf --]
[-- Type: text/x-vcard, Size: 74 bytes --]
begin:vcard
fn:AJAY KV
n:;AJAY
tel;cell:8892753703
version:2.1
end:vcard
^ permalink raw reply
* BLE multiple connection - Recv() issue
From: Ajay @ 2013-01-02 18:37 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <50DF97FB.7080700@globaledgesoft.com>
[-- Attachment #1: Type: text/plain, Size: 697 bytes --]
hi,
I have 2 LE links A1 to B and A2 to C ( A1 and A2 are client
running at same device )
when i starts sending data from B to A1 its happening and A1 is
receiving properly . But the very next moment i started data transfer
from C to A2 , A1 stops recievng . I could see data from B to A1
reaches upto hci layer(hcidump) , and then routed to A2 l2cap recv() .
Here i used recv() and send() system calls for data transfer with l2cap
socket (cid 0x04 and psm 0x00) .Bind() is done at both client and server
side . Is it possible for a BLE device to recv data simultaneously from
other two devices .????? .
--
Thanks & Regards
AJAY KV
GlobalEdge software Ltd
8892753703
[-- Attachment #2: ajay_kv.vcf --]
[-- Type: text/x-vcard, Size: 81 bytes --]
begin:vcard
fn:AJAY KV
n:;AJAY
tel;cell:8892753703
version:2.1
end:vcard
^ permalink raw reply
* [RFC] Bluetooth: Allow getting SCO options for not connected sockets
From: Vinicius Costa Gomes @ 2013-01-02 14:47 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Vinicius Costa Gomes
Now, that we have proper support for deferred setup for SCO sockets it
is very convenient that we are able to get the socket options for
sockets that are not in the 'connected' state.
And makes the behaviour more consistent with what L2CAP does, for
example.
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
---
Sending this as a RFC, because even though I couldn't think of any cases
that this new behaviour would surprise userspace applications, there
may be still be some.
net/bluetooth/sco.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 531a93d..271320c 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -730,11 +730,6 @@ static int sco_sock_getsockopt_old(struct socket *sock, int optname, char __user
switch (optname) {
case SCO_OPTIONS:
- if (sk->sk_state != BT_CONNECTED) {
- err = -ENOTCONN;
- break;
- }
-
opts.mtu = sco_pi(sk)->conn->mtu;
BT_DBG("mtu %d", opts.mtu);
--
1.8.0.2
^ permalink raw reply related
* Re: [PATCH] obexd: Fix crash while removing session
From: Luiz Augusto von Dentz @ 2013-01-02 14:27 UTC (permalink / raw)
To: Marcin Zawiejski; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1356964310.3001.1.camel@lsdevbox>
Hi Marcin,
On Mon, Dec 31, 2012 at 4:31 PM, Marcin Zawiejski <dragmz@gmail.com> wrote:
> On Mon, 2012-12-31 at 15:22 +0100, Marcin Zawiejski wrote:
>> Crash occurs when removing a session with RemoveSession while another session has been created but not yet registered
>>
>
> Sorry for double post, please ignore this one.
>
> Marcin.
Pushed, thanks
--
Luiz Augusto von Dentz
^ permalink raw reply
* Re: [PATCH] Bluetooth: Fix uuid output in debugfs
From: Johan Hedberg @ 2013-01-02 13:43 UTC (permalink / raw)
To: Bjørn Mork, Gustavo Padovan, linux-bluetooth
In-Reply-To: <20130102113122.GA22512@x220>
Hi,
On Wed, Jan 02, 2013, Johan Hedberg wrote:
> > > + u32 data0, data5;
> > > + u16 data1, data2, data3, data4;
> > > +
> > > + data5 = le32_to_cpu(*(__le32 *)uuid);
> > > + data4 = le16_to_cpu(*(__le16 *)(uuid + 4));
> > > + data3 = le16_to_cpu(*(__le16 *)(uuid + 6));
> > > + data2 = le16_to_cpu(*(__le16 *)(uuid + 8));
> > > + data1 = le16_to_cpu(*(__le16 *)(uuid + 10));
> > > + data0 = le32_to_cpu(*(__le32 *)(uuid + 12));
> > > +
> > > + seq_printf(f, "%.8x-%.4x-%.4x-%.4x-%.4x%.8x\n",
> > > + data0, data1, data2, data3, data4, data5);
> > > }
> > >
> > > static int uuids_show(struct seq_file *f, void *p)
> >
> >
> > Why can't all this be replaced with
> >
> > static void print_bt_uuid(struct seq_file *f, u8 *uuid)
> > {
> > seq_printf(f, "%pUl\n", uuid);
> > }
> >
> > ?
>
> I don't think there's any reason assuming that there are no unaligned
> access considerations (which I pointed out in my other reply). I wasn't
> aware of printk having such a nice extension to the usual format
> specifiers (and neither was Gustavo as it seems). Thanks for making us
> aware of it!
Actually I'm not getting the expected results with %pU. The following is
the output of seq_printf(f, "%pUl %pUb\n", uuid, uuid):
$ cat /sys/kernel/debug/bluetooth/hci0/uuids
5f9b34fb-0080-8000-0010-00000e110000 fb349b5f-8000-0080-0010-00000e110000
5f9b34fb-0080-8000-0010-00000c110000 fb349b5f-8000-0080-0010-00000c110000
5f9b34fb-0080-8000-0010-000004a00000 fb349b5f-8000-0080-0010-000004a00000
5f9b34fb-0080-8000-0010-000001180000 fb349b5f-8000-0080-0010-000001180000
5f9b34fb-0080-8000-0010-000000180000 fb349b5f-8000-0080-0010-000000180000
5f9b34fb-0080-8000-0010-000000120000 fb349b5f-8000-0080-0010-000000120000
None of those are of the correct format which I do get with Gustavo's
patch:
$ cat /sys/kernel/debug/bluetooth/hci0/uuids
0000110e-0000-1000-8000-00805f9b34fb
0000110c-0000-1000-8000-00805f9b34fb
0000a004-0000-1000-8000-00805f9b34fb
00001801-0000-1000-8000-00805f9b34fb
00001800-0000-1000-8000-00805f9b34fb
00001200-0000-1000-8000-00805f9b34fb
So it seems to me that the %pU specifier is expecting something quite
different from how we store the UUIDs. Looking at the difference of %pUb
and %pUl it seems it doesn't do a complete byte order swap for the whole
value but only for half of it, and even that half doesn't see a proper
64-bit byte order swap but just internal swaps for the one 32-bit and
two 16-bit parts.
So assuming that potential alignment issues get sorted out it seems that
Gustavos patch is the way to go forward.
Johan
^ permalink raw reply
* Re: [PATCH] Bluetooth: Fix uuid output in debugfs
From: Bjørn Mork @ 2013-01-02 12:37 UTC (permalink / raw)
To: Gustavo Padovan; +Cc: linux-bluetooth
In-Reply-To: <20130102113122.GA22512@x220>
Johan Hedberg <johan.hedberg@gmail.com> writes:
>> Why can't all this be replaced with
>>
>> static void print_bt_uuid(struct seq_file *f, u8 *uuid)
>> {
>> seq_printf(f, "%pUl\n", uuid);
>> }
>>
>> ?
>
> I don't think there's any reason assuming that there are no unaligned
> access considerations (which I pointed out in my other reply). I wasn't
> aware of printk having such a nice extension to the usual format
> specifiers (and neither was Gustavo as it seems). Thanks for making us
> aware of it!
lib/vsprintf.c:uuid_string() will only access "uuid" byte-by-byte so
alignment shouldn't be a problem.
Bjørn
^ permalink raw reply
* Re: [PATCH] Bluetooth: Fix uuid output in debugfs
From: Johan Hedberg @ 2013-01-02 11:31 UTC (permalink / raw)
To: Bjørn Mork; +Cc: Gustavo Padovan, linux-bluetooth
In-Reply-To: <87623g9chw.fsf@nemi.mork.no>
Hi Bjørn,
On Wed, Jan 02, 2013, Bjørn Mork wrote:
> > diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
> > index 55cceee..05b78c7 100644
> > --- a/net/bluetooth/hci_sysfs.c
> > +++ b/net/bluetooth/hci_sysfs.c
> > @@ -461,19 +461,18 @@ static const struct file_operations blacklist_fops = {
> >
> > static void print_bt_uuid(struct seq_file *f, u8 *uuid)
> > {
> > - __be32 data0, data4;
> > - __be16 data1, data2, data3, data5;
> > -
> > - memcpy(&data0, &uuid[0], 4);
> > - memcpy(&data1, &uuid[4], 2);
> > - memcpy(&data2, &uuid[6], 2);
> > - memcpy(&data3, &uuid[8], 2);
> > - memcpy(&data4, &uuid[10], 4);
> > - memcpy(&data5, &uuid[14], 2);
> > -
> > - seq_printf(f, "%.8x-%.4x-%.4x-%.4x-%.8x%.4x\n",
> > - ntohl(data0), ntohs(data1), ntohs(data2), ntohs(data3),
> > - ntohl(data4), ntohs(data5));
> > + u32 data0, data5;
> > + u16 data1, data2, data3, data4;
> > +
> > + data5 = le32_to_cpu(*(__le32 *)uuid);
> > + data4 = le16_to_cpu(*(__le16 *)(uuid + 4));
> > + data3 = le16_to_cpu(*(__le16 *)(uuid + 6));
> > + data2 = le16_to_cpu(*(__le16 *)(uuid + 8));
> > + data1 = le16_to_cpu(*(__le16 *)(uuid + 10));
> > + data0 = le32_to_cpu(*(__le32 *)(uuid + 12));
> > +
> > + seq_printf(f, "%.8x-%.4x-%.4x-%.4x-%.4x%.8x\n",
> > + data0, data1, data2, data3, data4, data5);
> > }
> >
> > static int uuids_show(struct seq_file *f, void *p)
>
>
> Why can't all this be replaced with
>
> static void print_bt_uuid(struct seq_file *f, u8 *uuid)
> {
> seq_printf(f, "%pUl\n", uuid);
> }
>
> ?
I don't think there's any reason assuming that there are no unaligned
access considerations (which I pointed out in my other reply). I wasn't
aware of printk having such a nice extension to the usual format
specifiers (and neither was Gustavo as it seems). Thanks for making us
aware of it!
Johan
^ permalink raw reply
* Re: Re: Specifying destination path in obexd/bluez-5
From: Alex Fiestas @ 2013-01-02 10:51 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZK6YAgZyxBa1a9kKFAHjy+dE7Td518CB393_9Y+RWPKkw@mail.gmail.com>
On Monday 31 December 2012 12:01:04 you wrote:
> Hi Alex,
>
> On Mon, Dec 31, 2012 at 10:56 AM, Alex Fiestas <afiestas@kde.org> wrote:
> > Last time I tried to port KDE's obex support completely to obexd (we are
> > already using obexd-client for opp sendfile) I couldn't complete it
> > because
> > there is no way of programatically specify the destination path of the
> > incoming files, iirc using either ftp or opp.
> >
> > Now checking out BlueZ-5 I'm not sure I see a way of doing it either.
>
> There is always the possibility of using links if the underline
> filesystem support them. For OPP the agent can provide a different
> path on the return of AuthorizePush.
Oh indeed, one thing less to worry then.
> > So, is there anyway of changing the destination path programatically
> > without having to restart obexd-server? If not, can I add a wish
> > somewhere (bugrack?) for it? Would you consider?
>
> I guess this is about FTP, right? If I understood you want to be able
> to change the folders being exported in run-time, that might be
> possible but if we do that I think it would be better to let the agent
> set its root folder once registered then you can fully control this on
> the component implementing the agent interface (usually the UI)
> leaving the command line switch only for systems without agents.
Only changing the root folder will be enough, we are already using links for
the rest.
With that, we should be able to port KDE bluetooth support to obexd-server
being the first step towards using BlueZ-5
Thanks !
^ permalink raw reply
* Re: [PATCH] Bluetooth: Fix uuid output in debugfs
From: Johan Hedberg @ 2013-01-02 9:27 UTC (permalink / raw)
To: Gustavo Padovan; +Cc: linux-bluetooth, Gustavo Padovan
In-Reply-To: <1356130695-29342-1-git-send-email-gustavo@padovan.org>
Hi Gustavo,
On Fri, Dec 21, 2012, Gustavo Padovan wrote:
> The uuid should be printed in the CPU endianness and not in little-endian.
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
> net/bluetooth/hci_sysfs.c | 25 ++++++++++++-------------
> 1 file changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
> index 55cceee..05b78c7 100644
> --- a/net/bluetooth/hci_sysfs.c
> +++ b/net/bluetooth/hci_sysfs.c
> @@ -461,19 +461,18 @@ static const struct file_operations blacklist_fops = {
>
> static void print_bt_uuid(struct seq_file *f, u8 *uuid)
> {
> - __be32 data0, data4;
> - __be16 data1, data2, data3, data5;
> -
> - memcpy(&data0, &uuid[0], 4);
> - memcpy(&data1, &uuid[4], 2);
> - memcpy(&data2, &uuid[6], 2);
> - memcpy(&data3, &uuid[8], 2);
> - memcpy(&data4, &uuid[10], 4);
> - memcpy(&data5, &uuid[14], 2);
> -
> - seq_printf(f, "%.8x-%.4x-%.4x-%.4x-%.8x%.4x\n",
> - ntohl(data0), ntohs(data1), ntohs(data2), ntohs(data3),
> - ntohl(data4), ntohs(data5));
> + u32 data0, data5;
> + u16 data1, data2, data3, data4;
> +
> + data5 = le32_to_cpu(*(__le32 *)uuid);
> + data4 = le16_to_cpu(*(__le16 *)(uuid + 4));
> + data3 = le16_to_cpu(*(__le16 *)(uuid + 6));
> + data2 = le16_to_cpu(*(__le16 *)(uuid + 8));
> + data1 = le16_to_cpu(*(__le16 *)(uuid + 10));
> + data0 = le32_to_cpu(*(__le32 *)(uuid + 12));
This looks prone to unaligned access violations if the "u8 *uuid"
pointer doesn't start off with a nicely aligned address. The use of the
unaligned getter macros would also look nicer since you wouldn't have to
do explicit type casting.
Johan
^ permalink raw reply
* Re: [PATCH] Bluetooth: Fix uuid output in debugfs
From: Bjørn Mork @ 2013-01-02 9:10 UTC (permalink / raw)
To: Gustavo Padovan; +Cc: linux-bluetooth
In-Reply-To: <1356130624-29234-1-git-send-email-gustavo@padovan.org>
> diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
> index 55cceee..05b78c7 100644
> --- a/net/bluetooth/hci_sysfs.c
> +++ b/net/bluetooth/hci_sysfs.c
> @@ -461,19 +461,18 @@ static const struct file_operations blacklist_fops = {
>
> static void print_bt_uuid(struct seq_file *f, u8 *uuid)
> {
> - __be32 data0, data4;
> - __be16 data1, data2, data3, data5;
> -
> - memcpy(&data0, &uuid[0], 4);
> - memcpy(&data1, &uuid[4], 2);
> - memcpy(&data2, &uuid[6], 2);
> - memcpy(&data3, &uuid[8], 2);
> - memcpy(&data4, &uuid[10], 4);
> - memcpy(&data5, &uuid[14], 2);
> -
> - seq_printf(f, "%.8x-%.4x-%.4x-%.4x-%.8x%.4x\n",
> - ntohl(data0), ntohs(data1), ntohs(data2), ntohs(data3),
> - ntohl(data4), ntohs(data5));
> + u32 data0, data5;
> + u16 data1, data2, data3, data4;
> +
> + data5 = le32_to_cpu(*(__le32 *)uuid);
> + data4 = le16_to_cpu(*(__le16 *)(uuid + 4));
> + data3 = le16_to_cpu(*(__le16 *)(uuid + 6));
> + data2 = le16_to_cpu(*(__le16 *)(uuid + 8));
> + data1 = le16_to_cpu(*(__le16 *)(uuid + 10));
> + data0 = le32_to_cpu(*(__le32 *)(uuid + 12));
> +
> + seq_printf(f, "%.8x-%.4x-%.4x-%.4x-%.4x%.8x\n",
> + data0, data1, data2, data3, data4, data5);
> }
>
> static int uuids_show(struct seq_file *f, void *p)
Why can't all this be replaced with
static void print_bt_uuid(struct seq_file *f, u8 *uuid)
{
seq_printf(f, "%pUl\n", uuid);
}
?
Bjørn
^ permalink raw reply
* Re: [PATCH 2/2] AVCTP: Replace calls to g_queue_free_full function
From: Marcel Holtmann @ 2013-01-02 1:19 UTC (permalink / raw)
To: Giovanni Gherdovich; +Cc: linux-bluetooth
In-Reply-To: <1357039308-8913-1-git-send-email-g.gherdovich@gmail.com>
Hi Giovanni,
> The function g_queue_free_full is available only from GLib 2.32.
> If BlueZ has to build against GLib 2.28, as stated in the configure.ac,
> this patch replaces the calls to g_queue_free_full in the AVTCP module
> with its body, taken from the sources of GLib 2.32.
> ---
> profiles/audio/avctp.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
> index 013c587..745ced8 100644
> --- a/profiles/audio/avctp.c
> +++ b/profiles/audio/avctp.c
> @@ -395,7 +395,8 @@ static void avctp_channel_destroy(struct avctp_channel *chan)
> g_source_remove(chan->process_id);
>
> g_free(chan->buffer);
> - g_queue_free_full(chan->queue, pending_destroy);
> + g_queue_foreach(chan->queue, (GFunc)pending_destroy, NULL);On Tue, 2013-01-01 at 12:21 +0100, Giovanni Gherdovich wrote:
Same here. Provide a proper pending_destroy. The other places can just
call it with NULL as second parameter.
> + g_queue_free(chan->queue);
> g_slist_free_full(chan->processed, pending_destroy);
> g_slist_free_full(chan->handlers, g_free);
> g_free(chan);
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/2] core: Replace calls to g_queue_free_full function
From: Marcel Holtmann @ 2013-01-02 1:18 UTC (permalink / raw)
To: Giovanni Gherdovich; +Cc: linux-bluetooth
In-Reply-To: <1357039294-8878-1-git-send-email-g.gherdovich@gmail.com>
Hi Giovanni,
> The function g_queue_free_full is available only from GLib 2.32.
> If BlueZ has to build against GLib 2.28, as stated in the configure.ac,
> this patch replaces the calls to g_queue_free_full in the "core" BlueZ module
> with its body, taken from the sources of GLib 2.32.
> ---
> src/adapter.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index e71cea8..73c9e58 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -1697,7 +1697,8 @@ static void adapter_free(gpointer user_data)
> if (adapter->auth_idle_id)
> g_source_remove(adapter->auth_idle_id);
>
> - g_queue_free_full(adapter->auths, g_free);
> + g_queue_foreach(adapter->auths, (GFunc)g_free, NULL);
I do not like the usage of (GFunc) casting here. Please add a function
that properly frees this and has the right signature.
> + g_queue_free(adapter->auths);
>
> sdp_list_free(adapter->services, NULL);
>
Regards
Marcel
^ permalink raw reply
* [PATCH 2/2] AVCTP: Replace calls to g_queue_free_full function
From: Giovanni Gherdovich @ 2013-01-01 11:21 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Giovanni Gherdovich
The function g_queue_free_full is available only from GLib 2.32.
If BlueZ has to build against GLib 2.28, as stated in the configure.ac,
this patch replaces the calls to g_queue_free_full in the AVTCP module
with its body, taken from the sources of GLib 2.32.
---
profiles/audio/avctp.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index 013c587..745ced8 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -395,7 +395,8 @@ static void avctp_channel_destroy(struct avctp_channel *chan)
g_source_remove(chan->process_id);
g_free(chan->buffer);
- g_queue_free_full(chan->queue, pending_destroy);
+ g_queue_foreach(chan->queue, (GFunc)pending_destroy, NULL);
+ g_queue_free(chan->queue);
g_slist_free_full(chan->processed, pending_destroy);
g_slist_free_full(chan->handlers, g_free);
g_free(chan);
--
1.7.4.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox