* [PATCH] Bluetooth: Send new settings event when changing high speed option
From: Marcel Holtmann @ 2013-10-01 5:17 UTC (permalink / raw)
To: linux-bluetooth
When enabling or disabling high speed setting it is required to send
a new settings event to inform other management interface users about
the changed settings.
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
net/bluetooth/mgmt.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 1b5b10f..13b5435 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1341,6 +1341,8 @@ failed:
static int set_hs(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
{
struct mgmt_mode *cp = data;
+ bool changed;
+ int err;
BT_DBG("request for %s", hdev->name);
@@ -1352,12 +1354,23 @@ static int set_hs(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
return cmd_status(sk, hdev->id, MGMT_OP_SET_HS,
MGMT_STATUS_INVALID_PARAMS);
+ hci_dev_lock(hdev);
+
if (cp->val)
- set_bit(HCI_HS_ENABLED, &hdev->dev_flags);
+ changed = !test_and_set_bit(HCI_HS_ENABLED, &hdev->dev_flags);
else
- clear_bit(HCI_HS_ENABLED, &hdev->dev_flags);
+ changed = test_and_clear_bit(HCI_HS_ENABLED, &hdev->dev_flags);
+
+ err = send_settings_rsp(sk, MGMT_OP_SET_HS, hdev);
+ if (err < 0)
+ goto unlock;
- return send_settings_rsp(sk, MGMT_OP_SET_HS, hdev);
+ if (changed)
+ err = new_settings(hdev, sk);
+
+unlock:
+ hci_dev_unlock(hdev);
+ return err;
}
static void le_enable_complete(struct hci_dev *hdev, u8 status)
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v2 3/3] android: Android version of log.c
From: Andrei Emeltchenko @ 2013-09-30 19:26 UTC (permalink / raw)
To: Frederic Danis; +Cc: Marcel Holtmann, Bluetooth Linux
In-Reply-To: <52498C8C.4070200@linux.intel.com>
[-- Attachment #1: Type: text/plain, Size: 836 bytes --]
Hi,
"Frederic Danis" <frederic.danis@linux.intel.com>
>
> Hello Marcel,
>
>
> On 27/09/2013 04:01, Marcel Holtmann wrote:
>>>
>>> Add logging system to BlueZ Android daemon.
>>> >Android build will use android/log.c file while autotools build will
use
>>> >src/log.c instead.
>>
>> lets just use stdout for now. Changing this later is pretty trivial and
introducing large build alternates is a bit too complicated at this point.
>
>
> I do not understand how writing code to log to stdout can be more simple
than building and linking existing code.
I agree with Frederic here. Why do we use error, warn, DBG, etc if we print
to stdout?
This actually will significantly slow log parsing since first you pay
attention to error messages. If all messages are printed with the same
priorities then we loose this ability.
Regards,
Andrei
[-- Attachment #2: Type: text/html, Size: 1129 bytes --]
^ permalink raw reply
* Re: [PATCH] lib: Fix use of uninitialized variable in sdp_set_profile_descs
From: Johan Hedberg @ 2013-09-30 18:19 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1380546363-10722-1-git-send-email-szymon.janc@tieto.com>
Hi Szymon,
On Mon, Sep 30, 2013, Szymon Janc wrote:
> Error path on default case was not breaking loop. To keep error
> handling similar all error path were converted to use goto.
>
> This fix following:
> target C: libbluetooth <= external/bluetooth/bluez/android/../lib/sdp.c
> lib/sdp.c: In function 'sdp_set_profile_descs':
> lib/sdp.c:487:10: warning: 'values[0]' may be used uninitialized in
> this function [-Wmaybe-uninitialized]
> lib/sdp.c:2562:19: note: 'values[0]' was declared here
> lib/sdp.c:545:11: warning: 'dtds[0]' may be used uninitialized in this
> function [-Wmaybe-uninitialized]
> lib/sdp.c:2562:9: note: 'dtds[0]' was declared here
> ---
> lib/sdp.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
Applied. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH 1/3] Bluetooth: Use only 2 bits for controller type information
From: Johan Hedberg @ 2013-09-30 18:16 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1380514261-21742-1-git-send-email-marcel@holtmann.org>
Hi Marcel,
On Sun, Sep 29, 2013, Marcel Holtmann wrote:
> The controller type is limited to BR/EDR/LE and AMP controllers. This
> can be easily encoded with just 2 bits and still leave enough room
> for future controller types.
>
> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> ---
> net/bluetooth/hci_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: Johan Hedberg <johan.hedberg@intel.com>
Johan
^ permalink raw reply
* Re: [PATCH 3/3] Bluetooth: Provide high speed configuration option
From: Johan Hedberg @ 2013-09-30 18:15 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1380514261-21742-3-git-send-email-marcel@holtmann.org>
Hi Marcel,
On Sun, Sep 29, 2013, Marcel Holtmann wrote:
> Hiding the Bluetooth high speed support behind a module parameter is
> not really useful. This can be enabled and disabled at runtime via
> the management interface. This also has the advantage that his can
> now be changed per controller and not just global.
>
> This patch removes the module parameter and exposes the high speed
> setting of the management interface to all controllers.
>
> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> ---
> include/net/bluetooth/l2cap.h | 1 +
> net/bluetooth/hci_core.c | 6 ------
> net/bluetooth/l2cap_core.c | 35 +++++++++++++++++++----------------
> net/bluetooth/l2cap_sock.c | 10 ----------
> net/bluetooth/mgmt.c | 11 ++---------
> 5 files changed, 22 insertions(+), 41 deletions(-)
Acked-by: Johan Hedberg <johan.hedberg@intel.com>
Johan
^ permalink raw reply
* Re: [PATCH v2 3/3] android: Android version of log.c
From: Frederic Danis @ 2013-09-30 14:37 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <7A4917A5-AD76-48AD-8C04-11707D64CE37@holtmann.org>
Hello Marcel,
On 27/09/2013 04:01, Marcel Holtmann wrote:
>> Add logging system to BlueZ Android daemon.
>> >Android build will use android/log.c file while autotools build will use
>> >src/log.c instead.
> lets just use stdout for now. Changing this later is pretty trivial and introducing large build alternates is a bit too complicated at this point.
I do not understand how writing code to log to stdout can be more simple
than building and linking existing code.
Regards
Fred
--
Frederic Danis Open Source Technology Center
frederic.danis@intel.com Intel Corporation
^ permalink raw reply
* Re: [PATCH v2 2/3] android: Add skeleton of BlueZ Android daemon
From: Frederic Danis @ 2013-09-30 14:36 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <2A24E0BD-2EFF-4811-833E-A850B1469054@holtmann.org>
Hello Marcel,
On 27/09/2013 04:05, Marcel Holtmann wrote:
>> Define local mapping to glib path, otherwise this has to be inside central
>> >place in the build repository.
>> >
>> >Retrieve Bluetooth version from configure.ac.
>> >---
>> >.gitignore | 2 +
>> >Makefile.android | 5 +++
> I rather keep the changes to a global Makefile separate from the android/ stuff.
>
>> >android/Android.mk | 24 ++++++++++++
>> >android/main.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>> >4 files changed, 141 insertions(+)
>> >create mode 100644 android/main.c
> It is perfectly fine to split this into two patches. Add the *.c file(s) first and in a second patch add the build changes.
>
Ok, so I will send .gitignore, Makefile.android and Android.mk in a
second patch.
Regards
Fred
--
Frederic Danis Open Source Technology Center
frederic.danis@intel.com Intel Corporation
^ permalink raw reply
* [PATCH] lib: Fix use of uninitialized variable in sdp_set_profile_descs
From: Szymon Janc @ 2013-09-30 13:06 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
Error path on default case was not breaking loop. To keep error
handling similar all error path were converted to use goto.
This fix following:
target C: libbluetooth <= external/bluetooth/bluez/android/../lib/sdp.c
lib/sdp.c: In function 'sdp_set_profile_descs':
lib/sdp.c:487:10: warning: 'values[0]' may be used uninitialized in
this function [-Wmaybe-uninitialized]
lib/sdp.c:2562:19: note: 'values[0]' was declared here
lib/sdp.c:545:11: warning: 'dtds[0]' may be used uninitialized in this
function [-Wmaybe-uninitialized]
lib/sdp.c:2562:9: note: 'dtds[0]' was declared here
---
lib/sdp.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index 54a99b6..1405c50 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -2546,6 +2546,7 @@ int sdp_set_profile_descs(sdp_record_t *rec, const sdp_list_t *profiles)
int i = 0, seqlen = sdp_list_len(profiles);
void **seqDTDs, **seqs;
const sdp_list_t *p;
+ sdp_data_t *pAPSeq;
seqDTDs = malloc(seqlen * sizeof(void *));
if (!seqDTDs)
@@ -2563,7 +2564,7 @@ int sdp_set_profile_descs(sdp_record_t *rec, const sdp_list_t *profiles)
sdp_profile_desc_t *profile = p->data;
if (!profile) {
status = -1;
- break;
+ goto end;
}
switch (profile->uuid.type) {
case SDP_UUID16:
@@ -2580,7 +2581,7 @@ int sdp_set_profile_descs(sdp_record_t *rec, const sdp_list_t *profiles)
break;
default:
status = -1;
- break;
+ goto end;
}
dtds[1] = &uint16;
values[1] = &profile->version;
@@ -2588,7 +2589,7 @@ int sdp_set_profile_descs(sdp_record_t *rec, const sdp_list_t *profiles)
if (seq == NULL) {
status = -1;
- break;
+ goto end;
}
seqDTDs[i] = &seq->dtd;
@@ -2596,10 +2597,10 @@ int sdp_set_profile_descs(sdp_record_t *rec, const sdp_list_t *profiles)
sdp_pattern_add_uuid(rec, &profile->uuid);
i++;
}
- if (status == 0) {
- sdp_data_t *pAPSeq = sdp_seq_alloc(seqDTDs, seqs, seqlen);
- sdp_attr_add(rec, SDP_ATTR_PFILE_DESC_LIST, pAPSeq);
- }
+
+ pAPSeq = sdp_seq_alloc(seqDTDs, seqs, seqlen);
+ sdp_attr_add(rec, SDP_ATTR_PFILE_DESC_LIST, pAPSeq);
+end:
free(seqDTDs);
free(seqs);
return status;
--
1.8.4
^ permalink raw reply related
* Re: [RFC 10/16] android: Add Android Makefile for libbluetooth
From: Szymon Janc @ 2013-09-30 12:14 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: Anderson Lizardo, BlueZ development
In-Reply-To: <20130930073222.GC10262@aemeltch-MOBL1>
Hi,
> > On Fri, Sep 27, 2013 at 10:12 AM, Andrei Emeltchenko
> > <Andrei.Emeltchenko.news@gmail.com> wrote:
> > > +# to suppress the "pointer of type 'void *' used in arithmetic" warning
> > > +LOCAL_CFLAGS += -Wno-pointer-arith
> > > +
> > > +# to suppress the "missing initializer near initialization" warning
> > > +LOCAL_CFLAGS += -Wno-missing-field-initializers
> > > +
> > > +# to suppress the "may be used uninitialized in this function" warning
> > > +LOCAL_CFLAGS += -Wno-maybe-uninitialized
> >
> > I wonder why you are suppressing all those warnings. Do you plan to
> > re-enable them once the implementation is more complete?
>
> I think most warnings are the same as in standard BlueZ build. At some
> point those issue must be resolved IMO.
There seems to be a bug in sdp_set_profile_descs() error path, for some reason
gcc 4.6 on my box doesn't generate warning, yet android one does.
Will send a patch for that in a moment.
--
Szymon Janc
^ permalink raw reply
* Re: [PATCH] Bluetooth: Use only 2 bits for controller type information
From: Anderson Lizardo @ 2013-09-30 11:31 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: BlueZ development
In-Reply-To: <4C2644C5-D946-4CB6-AEF5-3948DC241BA6@holtmann.org>
Hi Marcel,
On Mon, Sep 30, 2013 at 12:01 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
>>> - di.type = (hdev->bus & 0x0f) | (hdev->dev_type << 4);
>>> + di.type = (hdev->bus & 0x0f) | ((hdev->dev_type & 0x30) << 4);
>>
>> Shouldn't it be (hdev->dev_type & 0x03) here?
>
> yes, this it should be 0x03 here. My mistake.
Just a reminder that userspace needs fixing as well.
Best Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [RFC 15/16] android: Implement basic HAL server
From: Andrei Emeltchenko @ 2013-09-30 10:26 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: Anderson Lizardo, BlueZ development
In-Reply-To: <9C20AE4B-57C8-48A8-AE2B-80CE7E334C7D@holtmann.org>
Hi Marcel,
On Mon, Sep 30, 2013 at 10:33:54AM +0200, Marcel Holtmann wrote:
> >>> + /* Since daemon would be run also on host we have to grant perms */
> >>> + chmod(sock_path, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH |
> >>> + S_IWOTH);
> >>
> >> If it is really necessary to have a world writable socket, better
> >> explain in detail on the comment.
> >
> > the problem here is that we need to run this on a host and on Android
> > where HAL is running under user bluetooth, this has to be changed to use
> > ifdef logic.
>
> and why are we not starting the daemon as user bluetooth with proper capabilities.
there is an option capability in init.rc
http://androidxref.com/4.3_r2.1/xref/system/core/init/init_parser.c#84
but it does nothing:
http://androidxref.com/4.3_r2.1/xref/system/core/init/init_parser.c#659
at least in Android 4.3
So we run as root and then drop uid and guid. Can we use some better way?
Best regards
Andrei Emeltchenko
^ permalink raw reply
* Re: [PATCH 3/3] Bluetooth: Provide high speed configuration option
From: Marcel Holtmann @ 2013-09-30 9:23 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <20130930091820.GH10262@aemeltch-MOBL1>
Hi Andrei,
>> Hiding the Bluetooth high speed support behind a module parameter is
>> not really useful. This can be enabled and disabled at runtime via
>> the management interface. This also has the advantage that his can
>
> typo here.
>
> …
the maintainer can fix that one up.
>> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
>> index c85537c..9119898 100644
>> --- a/net/bluetooth/l2cap_sock.c
>> +++ b/net/bluetooth/l2cap_sock.c
>> @@ -445,11 +445,6 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
>> break;
>>
>> case BT_CHANNEL_POLICY:
>> - if (!enable_hs) {
>> - err = -ENOPROTOOPT;
>> - break;
>> - }
>> -
>
> Do you think we don't need to check that HS is enabled here and below?
We can not. The socket might not be bound or the controller does not exists. So we have no way of knowing.
It should also work no matter what. The presence of an AMP controller is optional. That is why the policy is defined as BREDR_ONLY, BREDR_PREFERRED and AMP_PREFERRED. You can not force the AMP transport.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 3/3] Bluetooth: Provide high speed configuration option
From: Andrei Emeltchenko @ 2013-09-30 9:18 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1380514261-21742-3-git-send-email-marcel@holtmann.org>
Hi Marcel,
On Sun, Sep 29, 2013 at 09:11:01PM -0700, Marcel Holtmann wrote:
> Hiding the Bluetooth high speed support behind a module parameter is
> not really useful. This can be enabled and disabled at runtime via
> the management interface. This also has the advantage that his can
typo here.
...
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index c85537c..9119898 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -445,11 +445,6 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
> break;
>
> case BT_CHANNEL_POLICY:
> - if (!enable_hs) {
> - err = -ENOPROTOOPT;
> - break;
> - }
> -
Do you think we don't need to check that HS is enabled here and below?
> if (put_user(chan->chan_policy, (u32 __user *) optval))
> err = -EFAULT;
> break;
> @@ -720,11 +715,6 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
> break;
>
> case BT_CHANNEL_POLICY:
> - if (!enable_hs) {
> - err = -ENOPROTOOPT;
> - break;
> - }
> -
> if (get_user(opt, (u32 __user *) optval)) {
> err = -EFAULT;
> break;
...
Best regards
Andrei Emeltchenko
^ permalink raw reply
* Re: [RFC 15/16] android: Implement basic HAL server
From: Andrei Emeltchenko @ 2013-09-30 8:42 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <20130929145947.GA28937@x220.p-661hnu-f1>
Hi Johan,
On Sun, Sep 29, 2013 at 05:59:47PM +0300, Johan Hedberg wrote:
> Hi Andrei,
>
> On Fri, Sep 27, 2013, Andrei Emeltchenko wrote:
> > + sock = socket(AF_UNIX, SOCK_STREAM, 0);
> > + if (sock < 0) {
> > + error("%s: socket(): %s", __func__, strerror(errno));
> > + return sock;
> > + }
>
> This should be SOCK_SEQPACKET. It has been mentioned several times in
> our (offline) conversations and is also clearly stated in the IPC
> document we'll soon push upstream.
>
> > + /* Since daemon would be run also on host we have to grant perms */
> > + chmod(sock_path, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH |
> > + S_IWOTH);
>
> Is this really necessary? On the host (by which I presume you mean the
> system where you build BlueZ, i.e. a non-android system) couldn't the
> test tool that's used just exec the daemon and then you have the same
> context for both processes that need to access the socket.
We need to set userid for the daemon in Android to "bluetooth" and my patch
with this change cannot compile for the host as it was mentioned in the
review. So I will use #ifdef to separate Android and host.
>
> > @@ -646,11 +648,13 @@ int main(int argc, char *argv[])
> > exit(1);
> >
> > init_mgmt_interface();
> > + start_hal_srv("/dev/socket/bluez_hal");
>
> Isn't /var/run the right place for stuff like this? Alternatively we
> could also go with an abstract socket and then not care about any
> associations with the file system at all.
Android sockets should go to the standard Android sockets place, for the
host this can be any of your choice. /var/run ?
I will separate them by ifdef logic
Best regards
Andrei Emeltchenko
^ permalink raw reply
* Re: [RFC 15/16] android: Implement basic HAL server
From: Marcel Holtmann @ 2013-09-30 8:33 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: Anderson Lizardo, BlueZ development
In-Reply-To: <20130930082507.GF10262@aemeltch-MOBL1>
Hi Andrei,
>>> +static gboolean io_session_event(GIOChannel *chan, GIOCondition cond,
>>> + gpointer data)
>>> +{
>>> + struct hal_msg_hdr hdr;
>>> + struct hal_msg_rsp rsp;
>>> + uint8_t *buf;
>>> + int sock, len, size;
>>> + uint8_t status;
>>> +
>>> + if (cond & G_IO_NVAL)
>>> + return FALSE;
>>> +
>>> + sock = g_io_channel_unix_get_fd(chan);
>>> +
>>> + if (cond & (G_IO_HUP | G_IO_ERR)) {
>>> + error("%s: error condition %d", __func__, cond);
>>> + /* TODO: handle */
>>> + return FALSE;
>>> + }
>>> +
>>> + len = recv(sock, &hdr, sizeof(hdr), MSG_PEEK);
>>> + if (len != sizeof(hdr)) {
>>> + /* TODO: handle */
>>> + return FALSE;
>>> + }
>>> +
>>> + size = sizeof(hdr) + hdr.len;
>>> + buf = malloc(size);
>>> + if (!buf)
>>> + return TRUE;
>>> +
>>> + len = recv(sock, buf, size, 0);
>>> + if (len != size) {
>>> + /* TODO: handle */
>>> + free(buf);
>>> + return FALSE;
>>> + }
>>> +
>>> + status = process_hal_msg(buf, len);
>>> +
>>> + free(buf);
>>> +
>>> + memcpy(&rsp, &hdr, sizeof(hdr));
>>> + rsp.hdr.len = sizeof(uint8_t);
>>> + rsp.status = status;
>>
>> You are not using "rsp" on this function (and not TODO comment explaining why).
>
> will fix this.
>
>>
>>> + /* Since daemon would be run also on host we have to grant perms */
>>> + chmod(sock_path, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH |
>>> + S_IWOTH);
>>
>> If it is really necessary to have a world writable socket, better
>> explain in detail on the comment.
>
> the problem here is that we need to run this on a host and on Android
> where HAL is running under user bluetooth, this has to be changed to use
> ifdef logic.
and why are we not starting the daemon as user bluetooth with proper capabilities.
Regards
Marcel
^ permalink raw reply
* Re: [RFC 15/16] android: Implement basic HAL server
From: Andrei Emeltchenko @ 2013-09-30 8:25 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: BlueZ development
In-Reply-To: <CAJdJm_PMy6pb-NnCc5fKc11ATUwKbSd71HMAWueWkxP_x4Ym=A@mail.gmail.com>
Hi Anderson,
On Fri, Sep 27, 2013 at 01:50:22PM -0400, Anderson Lizardo wrote:
> Hi Andrei,
>
> On Fri, Sep 27, 2013 at 10:12 AM, Andrei Emeltchenko
> <Andrei.Emeltchenko.news@gmail.com> wrote:
> > +static gboolean io_session_event(GIOChannel *chan, GIOCondition cond,
> > + gpointer data)
> > +{
> > + struct hal_msg_hdr hdr;
> > + struct hal_msg_rsp rsp;
> > + uint8_t *buf;
> > + int sock, len, size;
> > + uint8_t status;
> > +
> > + if (cond & G_IO_NVAL)
> > + return FALSE;
> > +
> > + sock = g_io_channel_unix_get_fd(chan);
> > +
> > + if (cond & (G_IO_HUP | G_IO_ERR)) {
> > + error("%s: error condition %d", __func__, cond);
> > + /* TODO: handle */
> > + return FALSE;
> > + }
> > +
> > + len = recv(sock, &hdr, sizeof(hdr), MSG_PEEK);
> > + if (len != sizeof(hdr)) {
> > + /* TODO: handle */
> > + return FALSE;
> > + }
> > +
> > + size = sizeof(hdr) + hdr.len;
> > + buf = malloc(size);
> > + if (!buf)
> > + return TRUE;
> > +
> > + len = recv(sock, buf, size, 0);
> > + if (len != size) {
> > + /* TODO: handle */
> > + free(buf);
> > + return FALSE;
> > + }
> > +
> > + status = process_hal_msg(buf, len);
> > +
> > + free(buf);
> > +
> > + memcpy(&rsp, &hdr, sizeof(hdr));
> > + rsp.hdr.len = sizeof(uint8_t);
> > + rsp.status = status;
>
> You are not using "rsp" on this function (and not TODO comment explaining why).
will fix this.
>
> > + /* Since daemon would be run also on host we have to grant perms */
> > + chmod(sock_path, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH |
> > + S_IWOTH);
>
> If it is really necessary to have a world writable socket, better
> explain in detail on the comment.
the problem here is that we need to run this on a host and on Android
where HAL is running under user bluetooth, this has to be changed to use
ifdef logic.
Best regards
Andrei Emeltchenko
^ permalink raw reply
* Re: [RFC 12/16] android: Add cap to bind to port < 1024
From: Andrei Emeltchenko @ 2013-09-30 7:51 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <7348700.bQ9Re4jemt@uw000953>
Hi Szymon,
On Mon, Sep 30, 2013 at 09:36:54AM +0200, Szymon Janc wrote:
> Hi Andrei,
>
> > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> >
> > For SDP server we need to bind to lower port, acquire this capability.
> > ---
> > android/main.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > configure.ac | 4 ++++
> > 2 files changed, 57 insertions(+)
> >
> > diff --git a/android/main.c b/android/main.c
> > index 5fef095..649867d 100644
> > --- a/android/main.c
> > +++ b/android/main.c
> > @@ -31,6 +31,19 @@
> > #include <stdio.h>
> > #include <stdlib.h>
> > #include <string.h>
> > +#include <unistd.h>
> > +#include <errno.h>
> > +#include <sys/prctl.h>
> > +#include <linux/capability.h>
> > +
> > +/**
> > + * Include <sys/capability.h> for host build and
> > + * also for Android 4.3 when it is added to bionic
> > + */
> > +#if (defined(__ANDROID_API__) && (__ANDROID_API__ > 17)) || \
> > + !defined(__ANDROID_API__)
> > +#include <sys/capability.h>
> > +#endif
> >
> > #include <glib.h>
> >
> > @@ -319,6 +332,43 @@ static void cleanup_mgmt_interface(void)
> > mgmt_if = NULL;
> > }
> >
> > +static bool android_set_aid_and_cap()
> > +{
> > + struct __user_cap_header_struct header;
> > + struct __user_cap_data_struct cap;
> > +
> > + DBG("%s: pid %d uid %d gid %d", __func__, getpid(), getuid(), getgid());
>
> DBG macro already adds function name to string so there is no need to double
> that. This applies to other places as well.
Yes, I forgot to remove that when changed debug to DBG.
Best regards
Andrei Emeltchenko
^ permalink raw reply
* Re: [RFC 11/16] android: sdp: Reuse BlueZ SDP server in Android
From: Andrei Emeltchenko @ 2013-09-30 7:47 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <B13A32F7-6E42-43FA-B821-D1815119ABBC@holtmann.org>
Hi Marcel,
On Sun, Sep 29, 2013 at 03:31:24PM +0200, Marcel Holtmann wrote:
> Hi Andrei,
>
> > Reuse existing SDP server code in Android GPL daemon.
> > ---
> > Makefile.android | 7 +++++--
> > android/Android.mk | 6 ++++++
> > android/bt_adapter.c | 5 ++++-
> > android/main.c | 27 +++++++++++++++++++++++++++
> > android/main.h | 25 +++++++++++++++++++++++++
> > 5 files changed, 67 insertions(+), 3 deletions(-)
> > create mode 100644 android/main.h
> >
> > diff --git a/Makefile.android b/Makefile.android
> > index 3e6fec0..bf82928 100644
> > --- a/Makefile.android
> > +++ b/Makefile.android
> > @@ -3,7 +3,10 @@ if ANDROID_DAEMON
> > noinst_PROGRAMS += android/bluezd
> >
> > android_bluezd_SOURCES = android/main.c src/log.c \
> > + src/sdpd-database.c src/sdpd-server.c \
> > + src/sdpd-service.c src/sdpd-request.c \
> > src/shared/util.h src/shared/util.c \
> > - src/shared/mgmt.h src/shared/mgmt.c
> > -android_bluezd_LDADD = @GLIB_LIBS@
> > + src/shared/mgmt.h src/shared/mgmt.c \
> > + android/bt_adapter.h android/bt_adapter.c
> > +android_bluezd_LDADD = lib/libbluetooth-internal.la @GLIB_LIBS@
> > endif
> > diff --git a/android/Android.mk b/android/Android.mk
> > index 36f8798..08e35e4 100644
> > --- a/android/Android.mk
> > +++ b/android/Android.mk
> > @@ -9,6 +9,11 @@ include $(CLEAR_VARS)
> > LOCAL_SRC_FILES := \
> > log.c \
> > main.c \
> > + bt_adapter.c \
> > + ../src/sdpd-database.c \
> > + ../src/sdpd-service.c \
> > + ../src/sdpd-request.c \
> > + ../src/sdpd-server.c \
> >
> > LOCAL_C_INCLUDES := \
> > $(call include-path-for, glib) \
> > @@ -24,6 +29,7 @@ LOCAL_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\"
> > LOCAL_SHARED_LIBRARIES := \
> > libglib \
> > libbluez \
> > + libbluetooth \
> >
> > LOCAL_MODULE := bluezd
> >
> > diff --git a/android/bt_adapter.c b/android/bt_adapter.c
> > index 9f64839..42facb6 100644
> > --- a/android/bt_adapter.c
> > +++ b/android/bt_adapter.c
> > @@ -23,6 +23,7 @@
> >
> > #include "bt_adapter.h"
> > #include "log.h"
> > +#include "main.h"
> > #include "src/shared/mgmt.h"
> >
> > struct bt_device *bt_device_ref(struct bt_device *device)
> > @@ -118,7 +119,7 @@ void adapter_start(struct bt_adapter *adapter)
> >
> > /* TODO: CB: report scan mode */
> >
> > - /* TODO: SDP start here */
> > + sdp_start();
> >
> > /* TODO: CB: report state on */
> > }
> > @@ -126,4 +127,6 @@ void adapter_start(struct bt_adapter *adapter)
> > void adapter_stop(struct bt_adapter *adapter)
> > {
> > DBG("disabled %u", adapter->dev_id);
> > +
> > + sdp_stop();
> > }
> > diff --git a/android/main.c b/android/main.c
> > index 4792919..5fef095 100644
> > --- a/android/main.c
> > +++ b/android/main.c
> > @@ -36,6 +36,8 @@
> >
> > #include "log.h"
> > #include "hcid.h"
> > +#include "sdpd.h"
> > +#include "main.h"
> >
> > #include "lib/bluetooth.h"
> > #include "lib/mgmt.h"
> > @@ -43,12 +45,37 @@
> >
> > #define SHUTDOWN_GRACE_SECONDS 10
> >
> > +struct main_opts main_opts;
> > +
>
> where is this main_opts coming from. I rather not have this.
Otherwise I get error:
sdpd-server.c:241: error: undefined reference to 'main_opts'
collect2: ld returned 1 exit status
BTW: Is this now fixed?
> > static GMainLoop *event_loop;
> > static struct mgmt *mgmt_if = NULL;
> >
> > static uint8_t mgmt_version = 0;
> > static uint8_t mgmt_revision = 0;
> >
> > +GList *adapter_list = NULL;
> > +struct bt_adapter *default_adapter = NULL;
>
> Why are this public variables.
This is the default adapter, it probably needs to be public, otherwise we
have to squash all daemon code to single main.c
> > +
> > +int sdp_start(void)
> > +{
> > + uint16_t mtu = 0;
> > + uint32_t flags = 0;
> > +
> > + DBG("");
> > +
> > + /* sdpd-server use these settings */
> > + memset(&main_opts, 0, sizeof(main_opts));
> > +
> > + return start_sdp_server(mtu, flags);
>
> Just fill in mtu and flags manually here. No need for variable declaration above.
OK
Best regards
Andrei Emeltchenko
^ permalink raw reply
* Re: [RFC 11/16] android: sdp: Reuse BlueZ SDP server in Android
From: Szymon Janc @ 2013-09-30 7:45 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: Andrei Emeltchenko, linux-bluetooth
In-Reply-To: <B13A32F7-6E42-43FA-B821-D1815119ABBC@holtmann.org>
Hi Marcel,
> > Reuse existing SDP server code in Android GPL daemon.
> > ---
> > Makefile.android | 7 +++++--
> > android/Android.mk | 6 ++++++
> > android/bt_adapter.c | 5 ++++-
> > android/main.c | 27 +++++++++++++++++++++++++++
> > android/main.h | 25 +++++++++++++++++++++++++
> > 5 files changed, 67 insertions(+), 3 deletions(-)
> > create mode 100644 android/main.h
> >
> > diff --git a/Makefile.android b/Makefile.android
> > index 3e6fec0..bf82928 100644
> > --- a/Makefile.android
> > +++ b/Makefile.android
> > @@ -3,7 +3,10 @@ if ANDROID_DAEMON
> > noinst_PROGRAMS += android/bluezd
> >
> > android_bluezd_SOURCES = android/main.c src/log.c \
> > + src/sdpd-database.c src/sdpd-server.c \
> > + src/sdpd-service.c src/sdpd-request.c \
> > src/shared/util.h src/shared/util.c \
> > - src/shared/mgmt.h src/shared/mgmt.c
> > -android_bluezd_LDADD = @GLIB_LIBS@
> > + src/shared/mgmt.h src/shared/mgmt.c \
> > + android/bt_adapter.h android/bt_adapter.c
> > +android_bluezd_LDADD = lib/libbluetooth-internal.la @GLIB_LIBS@
> > endif
> > diff --git a/android/Android.mk b/android/Android.mk
> > index 36f8798..08e35e4 100644
> > --- a/android/Android.mk
> > +++ b/android/Android.mk
> > @@ -9,6 +9,11 @@ include $(CLEAR_VARS)
> > LOCAL_SRC_FILES := \
> > log.c \
> > main.c \
> > + bt_adapter.c \
> > + ../src/sdpd-database.c \
> > + ../src/sdpd-service.c \
> > + ../src/sdpd-request.c \
> > + ../src/sdpd-server.c \
> >
> > LOCAL_C_INCLUDES := \
> > $(call include-path-for, glib) \
> > @@ -24,6 +29,7 @@ LOCAL_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\"
> > LOCAL_SHARED_LIBRARIES := \
> > libglib \
> > libbluez \
> > + libbluetooth \
> >
> > LOCAL_MODULE := bluezd
> >
> > diff --git a/android/bt_adapter.c b/android/bt_adapter.c
> > index 9f64839..42facb6 100644
> > --- a/android/bt_adapter.c
> > +++ b/android/bt_adapter.c
> > @@ -23,6 +23,7 @@
> >
> > #include "bt_adapter.h"
> > #include "log.h"
> > +#include "main.h"
> > #include "src/shared/mgmt.h"
> >
> > struct bt_device *bt_device_ref(struct bt_device *device)
> > @@ -118,7 +119,7 @@ void adapter_start(struct bt_adapter *adapter)
> >
> > /* TODO: CB: report scan mode */
> >
> > - /* TODO: SDP start here */
> > + sdp_start();
> >
> > /* TODO: CB: report state on */
> > }
> > @@ -126,4 +127,6 @@ void adapter_start(struct bt_adapter *adapter)
> > void adapter_stop(struct bt_adapter *adapter)
> > {
> > DBG("disabled %u", adapter->dev_id);
> > +
> > + sdp_stop();
> > }
> > diff --git a/android/main.c b/android/main.c
> > index 4792919..5fef095 100644
> > --- a/android/main.c
> > +++ b/android/main.c
> > @@ -36,6 +36,8 @@
> >
> > #include "log.h"
> > #include "hcid.h"
> > +#include "sdpd.h"
> > +#include "main.h"
> >
> > #include "lib/bluetooth.h"
> > #include "lib/mgmt.h"
> > @@ -43,12 +45,37 @@
> >
> > #define SHUTDOWN_GRACE_SECONDS 10
> >
> > +struct main_opts main_opts;
> > +
>
> where is this main_opts coming from. I rather not have this.
It is from deviceid related code in sdpd-server.
We could pass device id info to start_sdp_server() to avoid that.
Or have some more general interface for accessing config options - Luiz
suggested ofono way (which is quite elegant imho).
--
BR
Szymon Janc
^ permalink raw reply
* Re: [RFC 12/16] android: Add cap to bind to port < 1024
From: Szymon Janc @ 2013-09-30 7:36 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1380291161-10232-13-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andrei,
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> For SDP server we need to bind to lower port, acquire this capability.
> ---
> android/main.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> configure.ac | 4 ++++
> 2 files changed, 57 insertions(+)
>
> diff --git a/android/main.c b/android/main.c
> index 5fef095..649867d 100644
> --- a/android/main.c
> +++ b/android/main.c
> @@ -31,6 +31,19 @@
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> +#include <unistd.h>
> +#include <errno.h>
> +#include <sys/prctl.h>
> +#include <linux/capability.h>
> +
> +/**
> + * Include <sys/capability.h> for host build and
> + * also for Android 4.3 when it is added to bionic
> + */
> +#if (defined(__ANDROID_API__) && (__ANDROID_API__ > 17)) || \
> + !defined(__ANDROID_API__)
> +#include <sys/capability.h>
> +#endif
>
> #include <glib.h>
>
> @@ -319,6 +332,43 @@ static void cleanup_mgmt_interface(void)
> mgmt_if = NULL;
> }
>
> +static bool android_set_aid_and_cap()
> +{
> + struct __user_cap_header_struct header;
> + struct __user_cap_data_struct cap;
> +
> + DBG("%s: pid %d uid %d gid %d", __func__, getpid(), getuid(), getgid());
DBG macro already adds function name to string so there is no need to double
that. This applies to other places as well.
> +
> + header.version = _LINUX_CAPABILITY_VERSION;
> + header.pid = getpid();
> + if (capget(&header, &cap) < 0)
> + error("%s: capget(): %s", __func__, strerror(errno));
> +
> + DBG("%s: Cap data 0x%x, 0x%x, 0x%x\n", __func__, cap.effective,
> + cap.permitted, cap.inheritable);
> +
> + prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0);
> +
> + header.version = _LINUX_CAPABILITY_VERSION;
> + header.pid = 0;
> +
> + cap.effective = cap.permitted = cap.inheritable =
> + 1 << CAP_NET_RAW |
> + 1 << CAP_NET_ADMIN |
> + 1 << CAP_NET_BIND_SERVICE |
> + 1 << CAP_SYS_RAWIO |
> + 1 << CAP_SYS_NICE |
> + 1 << CAP_SETGID;
> +
> + if (capset(&header, &cap)) {
> + error("%s: capset(): %s", __func__, strerror(errno));
> + return false;
> + }
> +
> + DBG("%s: capset(): Success", __func__);
> + return true;
> +}
> +
> int main(int argc, char *argv[])
> {
> GOptionContext *context;
> @@ -357,6 +407,9 @@ int main(int argc, char *argv[])
> /* no need to keep parsed option in memory */
> free_options();
>
> + if (android_set_aid_and_cap() == false)
> + exit(1);
> +
> init_mgmt_interface();
>
> DBG("Entering main loop");
> diff --git a/configure.ac b/configure.ac
> index 3b7a5d9..af418d3 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -247,4 +247,8 @@ AC_ARG_ENABLE(android-daemon, AC_HELP_STRING([--enable-android-daemon],
> [android_daemon=${enableval}])
> AM_CONDITIONAL(ANDROID_DAEMON, test "${android_daemon}" = "yes")
>
> +if (test "${android_daemon}" = "yes"); then
> + AC_CHECK_LIB(cap, capget, dummy=yes, AC_MSG_ERROR(libcap is required))
> +fi
> +
> AC_OUTPUT(Makefile src/bluetoothd.8 lib/bluez.pc)
>
--
BR
Szymon Janc
^ permalink raw reply
* Re: [RFC 10/16] android: Add Android Makefile for libbluetooth
From: Andrei Emeltchenko @ 2013-09-30 7:32 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: BlueZ development
In-Reply-To: <CAJdJm_OYf-QjS9iS6vON7jw8biq0_8g0KpoHBW_-Gb0-U6w3Pg@mail.gmail.com>
Hi Anderson,
On Fri, Sep 27, 2013 at 02:01:31PM -0400, Anderson Lizardo wrote:
> Hi Andrei,
>
> On Fri, Sep 27, 2013 at 10:12 AM, Andrei Emeltchenko
> <Andrei.Emeltchenko.news@gmail.com> wrote:
> > +# to suppress the "pointer of type 'void *' used in arithmetic" warning
> > +LOCAL_CFLAGS += -Wno-pointer-arith
> > +
> > +# to suppress the "missing initializer near initialization" warning
> > +LOCAL_CFLAGS += -Wno-missing-field-initializers
> > +
> > +# to suppress the "may be used uninitialized in this function" warning
> > +LOCAL_CFLAGS += -Wno-maybe-uninitialized
>
> I wonder why you are suppressing all those warnings. Do you plan to
> re-enable them once the implementation is more complete?
I think most warnings are the same as in standard BlueZ build. At some
point those issue must be resolved IMO.
Best regards
Andrei Emeltchenko
^ permalink raw reply
* Re: [RFC 09/16] android: Add adapter and device struct for BlueZ daemon
From: Andrei Emeltchenko @ 2013-09-30 7:28 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <C606466F-9DC7-45F0-A73C-FA7940DD7CF1@holtmann.org>
Hi Marcel,
On Sun, Sep 29, 2013 at 03:38:39PM +0200, Marcel Holtmann wrote:
> Hi Andrei,
>
> > Adapter structure in BlueZ daemon keeps track of default adapter
> > and device structure keeps track about found devices.
> > ---
> > android/bt_adapter.c | 129 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > android/bt_adapter.h | 68 ++++++++++++++++++++++++++
> > 2 files changed, 197 insertions(+)
> > create mode 100644 android/bt_adapter.c
> > create mode 100644 android/bt_adapter.h
> >
> > diff --git a/android/bt_adapter.c b/android/bt_adapter.c
> > new file mode 100644
> > index 0000000..9f64839
> > --- /dev/null
> > +++ b/android/bt_adapter.c
> > @@ -0,0 +1,129 @@
> > +/*
> > + *
> > + * BlueZ - Bluetooth protocol stack for Linux
> > + *
> > + * Copyright (C) 2013 Intel Corporation. All rights reserved.
> > + *
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> > + *
> > + */
> > +
> > +#include "bt_adapter.h"
> > +#include "log.h"
> > +#include "src/shared/mgmt.h"
> > +
> > +struct bt_device *bt_device_ref(struct bt_device *device)
> > +{
> > + DBG("");
> > +
> > + __sync_fetch_and_add(&device->refcnt, 1);
> > +
> > + return device;
> > +}
> > +
> > +void bt_device_unref(struct bt_device *device)
> > +{
> > + DBG("");
> > +
> > + if (__sync_sub_and_fetch(&device->refcnt, 1))
> > + return;
> > +
> > + /* TODO: */
> > + DBG("%s: Freeing device %p name %s", __func__, device, device->name);
> > +
> > + free(device->name);
> > + free(device);
> > +}
> > +
> > +struct bt_adapter *bt_adapter_lookup(uint16_t index)
> > +{
> > + GList *list;
> > +
> > + for (list = g_list_first(adapter_list); list;
> > + list = g_list_next(list)) {
> > + struct bt_adapter *adapter = list->data;
> > +
> > + if (adapter->dev_id == index)
> > + return adapter;
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > +struct bt_adapter *bt_adapter_ref(struct bt_adapter *adapter)
> > +{
> > + DBG("");
> > +
> > + __sync_fetch_and_add(&adapter->refcnt, 1);
> > +
> > + return adapter;
> > +}
> > +
> > +static void bt_adapter_free(struct bt_adapter *adapter)
> > +{
> > + DBG("");
> > +
> > + while (adapter->found_devices) {
> > + struct bt_device *device = adapter->found_devices->data;
> > +
> > + adapter->found_devices = g_list_remove(adapter->found_devices,
> > + device);
> > + bt_device_unref(device);
> > + }
> > +
> > + mgmt_unref(adapter->mgmt);
> > + free(adapter);
> > +}
> > +
> > +void bt_adapter_unref(struct bt_adapter *adapter)
> > +{
> > + DBG("");
> > +
> > + if (__sync_sub_and_fetch(&adapter->refcnt, 1))
> > + return;
> > +
> > + bt_adapter_free(adapter);
> > +}
> > +
> > +struct bt_adapter *bt_adapter_new(uint16_t index, struct mgmt *mgmt_if)
> > +{
> > + struct bt_adapter *adapter;
> > +
> > + adapter = g_try_new0(struct bt_adapter, 1);
> > + if (!adapter)
> > + return NULL;
> > +
> > + adapter->dev_id = index;
> > + adapter->mgmt = mgmt_ref(mgmt_if);
> > +
> > + return bt_adapter_ref(adapter);
> > +}
> > +
> > +void adapter_start(struct bt_adapter *adapter)
> > +{
> > + DBG("enabled %u", adapter->dev_id);
> > +
> > + /* TODO: CB: report scan mode */
> > +
> > + /* TODO: SDP start here */
> > +
> > + /* TODO: CB: report state on */
> > +}
> > +
> > +void adapter_stop(struct bt_adapter *adapter)
> > +{
> > + DBG("disabled %u", adapter->dev_id);
> > +}
> > diff --git a/android/bt_adapter.h b/android/bt_adapter.h
> > new file mode 100644
> > index 0000000..5634729
> > --- /dev/null
> > +++ b/android/bt_adapter.h
> > @@ -0,0 +1,68 @@
> > +/*
> > + *
> > + * BlueZ - Bluetooth protocol stack for Linux
> > + *
> > + * Copyright (C) 2013 Intel Corporation. All rights reserved.
> > + *
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> > + *
> > + */
> > +
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <unistd.h>
> > +#include <glib.h>
> > +
> > +#include "lib/bluetooth.h"
> > +
> > +struct bt_device {
> > + int refcnt;
> > +
> > + bdaddr_t bdaddr;
> > + uint8_t bdaddr_type;
> > + uint32_t cod;
> > + char *name;
> > +};
> > +
> > +struct bt_adapter {
> > + int refcnt;
> > +
> > + uint16_t dev_id;
> > + struct mgmt *mgmt;
> > + bdaddr_t bdaddr;
> > + uint32_t dev_class;
> > +
> > + char *name;
> > + char *short_name;
> > +
> > + uint32_t supported_settings;
> > + uint32_t current_settings;
> > +
> > + GList *found_devices;
> > +};
> > +
> > +extern GList *adapter_list;
> > +
> > +struct bt_adapter *bt_adapter_lookup(uint16_t index);
> > +struct bt_adapter *bt_adapter_ref(struct bt_adapter *adapter);
> > +void bt_adapter_unref(struct bt_adapter *adapter);
> > +struct bt_adapter *bt_adapter_new(uint16_t index, struct mgmt *mgmt_if);
> > +
> > +struct bt_device *bt_device_ref(struct bt_device *device);
> > +void bt_device_unref(struct bt_device *device);
> > +
> > +void adapter_start(struct bt_adapter *adapter);
> > +void adapter_stop(struct bt_adapter *adapter);
>
> since we only support one single controller and will ignore all others,
> why are we building such a complicate abstraction. We could make this
> functionality part of the core daemon and no need to create an adapter
> abstraction.
>
> Just copying bluetoothd is not helping us here since at its core, it
> works way different. We do not need all that logic here.
I try to use pointer default_adapter which points to the first device
with index 0. For devices we still need some kind of list.
Best regards
Andrei Emeltchenko
^ permalink raw reply
* Re: [RFC 04/16] android: Start Android Bluetooth daemon
From: Andrei Emeltchenko @ 2013-09-30 7:07 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <9A70E0AA-C658-4B1D-8040-DD483CCD6047@holtmann.org>
Hi Marcel,
On Sun, Sep 29, 2013 at 03:28:44PM +0200, Marcel Holtmann wrote:
> Hi Andrei,
>
> > Start Android Bluetooth daemon from HAL init(). Make sure
> > that daemon is in "running" state.
> > ---
> > android/hal_bluetooth.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 53 insertions(+), 1 deletion(-)
> >
> > diff --git a/android/hal_bluetooth.c b/android/hal_bluetooth.c
> > index 5298618..036091e 100644
> > --- a/android/hal_bluetooth.c
> > +++ b/android/hal_bluetooth.c
> > @@ -23,11 +23,16 @@
> > #include <hardware/bluetooth.h>
> > #include <hardware/bt_sock.h>
> >
> > +#include <cutils/sockets.h>
> > +#include <cutils/properties.h>
> > +
> > #define LOG_TAG "BlueZ"
> > #include <cutils/log.h>
> >
> > #include "hal.h"
> >
> > +#define ANDROID_BLUEZ "bluezd"
> > +
>
> the name changes to either "btd" or "bluetoothd". We need to pick one, but it is not suppose to be "bluezd".
this is the name of the service as it named in init.rc, so if bluetoothd
is the name of the daemon executable then I name service as btd.
Maybe I change ANDROID_BLUEZ to ANDROID_BLUEZ_SVC ?
>
> > bt_callbacks_t *bt_hal_cbacks = NULL;
>
> This needs to be public?
Yes, they would be called from callback thread and, possibly, from here.
Best regards
Andrei Emeltchenko
^ permalink raw reply
* [PATCH 3/3] Bluetooth: Provide high speed configuration option
From: Marcel Holtmann @ 2013-09-30 4:11 UTC (permalink / raw)
To: linux-bluetooth
Hiding the Bluetooth high speed support behind a module parameter is
not really useful. This can be enabled and disabled at runtime via
the management interface. This also has the advantage that his can
now be changed per controller and not just global.
This patch removes the module parameter and exposes the high speed
setting of the management interface to all controllers.
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
include/net/bluetooth/l2cap.h | 1 +
net/bluetooth/hci_core.c | 6 ------
net/bluetooth/l2cap_core.c | 35 +++++++++++++++++++----------------
net/bluetooth/l2cap_sock.c | 10 ----------
net/bluetooth/mgmt.c | 11 ++---------
5 files changed, 22 insertions(+), 41 deletions(-)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 1a966af..f141b5f 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -564,6 +564,7 @@ struct l2cap_conn {
__u32 feat_mask;
__u8 fixed_chan_mask;
+ bool hs_enabled;
__u8 info_state;
__u8 info_ident;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index dd2528c5..750c360 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1222,12 +1222,6 @@ int hci_dev_open(__u16 dev)
ret = hdev->setup(hdev);
if (!ret) {
- /* Treat all non BR/EDR controllers as raw devices if
- * enable_hs is not set.
- */
- if (hdev->dev_type != HCI_BREDR && !enable_hs)
- set_bit(HCI_RAW, &hdev->flags);
-
if (test_bit(HCI_QUIRK_RAW_DEVICE, &hdev->quirks))
set_bit(HCI_RAW, &hdev->flags);
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index d1f1e78..6d42498 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1016,13 +1016,12 @@ static bool __amp_capable(struct l2cap_chan *chan)
{
struct l2cap_conn *conn = chan->conn;
- if (enable_hs &&
- hci_amp_capable() &&
+ if (conn->hs_enabled && hci_amp_capable() &&
chan->chan_policy == BT_CHANNEL_POLICY_AMP_PREFERRED &&
conn->fixed_chan_mask & L2CAP_FC_A2MP)
return true;
- else
- return false;
+
+ return false;
}
static bool l2cap_check_efs(struct l2cap_chan *chan)
@@ -1638,6 +1637,10 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon)
conn->feat_mask = 0;
+ if (hcon->type == ACL_LINK)
+ conn->hs_enabled = test_bit(HCI_HS_ENABLED,
+ &hcon->hdev->dev_flags);
+
spin_lock_init(&conn->lock);
mutex_init(&conn->chan_lock);
@@ -3084,14 +3087,14 @@ static inline __u8 l2cap_select_mode(__u8 mode, __u16 remote_feat_mask)
}
}
-static inline bool __l2cap_ews_supported(struct l2cap_chan *chan)
+static inline bool __l2cap_ews_supported(struct l2cap_conn *conn)
{
- return enable_hs && chan->conn->feat_mask & L2CAP_FEAT_EXT_WINDOW;
+ return conn->hs_enabled && conn->feat_mask & L2CAP_FEAT_EXT_WINDOW;
}
-static inline bool __l2cap_efs_supported(struct l2cap_chan *chan)
+static inline bool __l2cap_efs_supported(struct l2cap_conn *conn)
{
- return enable_hs && chan->conn->feat_mask & L2CAP_FEAT_EXT_FLOW;
+ return conn->hs_enabled && conn->feat_mask & L2CAP_FEAT_EXT_FLOW;
}
static void __l2cap_set_ertm_timeouts(struct l2cap_chan *chan,
@@ -3135,7 +3138,7 @@ static void __l2cap_set_ertm_timeouts(struct l2cap_chan *chan,
static inline void l2cap_txwin_setup(struct l2cap_chan *chan)
{
if (chan->tx_win > L2CAP_DEFAULT_TX_WINDOW &&
- __l2cap_ews_supported(chan)) {
+ __l2cap_ews_supported(chan->conn)) {
/* use extended control field */
set_bit(FLAG_EXT_CTRL, &chan->flags);
chan->tx_win_max = L2CAP_DEFAULT_EXT_WINDOW;
@@ -3165,7 +3168,7 @@ static int l2cap_build_conf_req(struct l2cap_chan *chan, void *data)
if (test_bit(CONF_STATE2_DEVICE, &chan->conf_state))
break;
- if (__l2cap_efs_supported(chan))
+ if (__l2cap_efs_supported(chan->conn))
set_bit(FLAG_EFS_ENABLE, &chan->flags);
/* fall through */
@@ -3317,7 +3320,7 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data)
break;
case L2CAP_CONF_EWS:
- if (!enable_hs)
+ if (!chan->conn->hs_enabled)
return -ECONNREFUSED;
set_bit(FLAG_EXT_CTRL, &chan->flags);
@@ -3349,7 +3352,7 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data)
}
if (remote_efs) {
- if (__l2cap_efs_supported(chan))
+ if (__l2cap_efs_supported(chan->conn))
set_bit(FLAG_EFS_ENABLE, &chan->flags);
else
return -ECONNREFUSED;
@@ -4303,7 +4306,7 @@ static inline int l2cap_information_req(struct l2cap_conn *conn,
if (!disable_ertm)
feat_mask |= L2CAP_FEAT_ERTM | L2CAP_FEAT_STREAMING
| L2CAP_FEAT_FCS;
- if (enable_hs)
+ if (conn->hs_enabled)
feat_mask |= L2CAP_FEAT_EXT_FLOW
| L2CAP_FEAT_EXT_WINDOW;
@@ -4314,7 +4317,7 @@ static inline int l2cap_information_req(struct l2cap_conn *conn,
u8 buf[12];
struct l2cap_info_rsp *rsp = (struct l2cap_info_rsp *) buf;
- if (enable_hs)
+ if (conn->hs_enabled)
l2cap_fixed_chan[0] |= L2CAP_FC_A2MP;
else
l2cap_fixed_chan[0] &= ~L2CAP_FC_A2MP;
@@ -4411,7 +4414,7 @@ static int l2cap_create_channel_req(struct l2cap_conn *conn,
if (cmd_len != sizeof(*req))
return -EPROTO;
- if (!enable_hs)
+ if (!conn->hs_enabled)
return -EINVAL;
psm = le16_to_cpu(req->psm);
@@ -4838,7 +4841,7 @@ static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
BT_DBG("icid 0x%4.4x, dest_amp_id %d", icid, req->dest_amp_id);
- if (!enable_hs)
+ if (!conn->hs_enabled)
return -EINVAL;
chan = l2cap_get_chan_by_dcid(conn, icid);
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index c85537c..9119898 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -445,11 +445,6 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
break;
case BT_CHANNEL_POLICY:
- if (!enable_hs) {
- err = -ENOPROTOOPT;
- break;
- }
-
if (put_user(chan->chan_policy, (u32 __user *) optval))
err = -EFAULT;
break;
@@ -720,11 +715,6 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
break;
case BT_CHANNEL_POLICY:
- if (!enable_hs) {
- err = -ENOPROTOOPT;
- break;
- }
-
if (get_user(opt, (u32 __user *) optval)) {
err = -EFAULT;
break;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 1b5b10f..dd15491 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -32,8 +32,6 @@
#include <net/bluetooth/mgmt.h>
#include <net/bluetooth/smp.h>
-bool enable_hs;
-
#define MGMT_VERSION 1
#define MGMT_REVISION 3
@@ -380,10 +378,8 @@ static u32 get_supported_settings(struct hci_dev *hdev)
settings |= MGMT_SETTING_DISCOVERABLE;
settings |= MGMT_SETTING_BREDR;
settings |= MGMT_SETTING_LINK_SECURITY;
- }
-
- if (enable_hs)
settings |= MGMT_SETTING_HS;
+ }
if (lmp_le_capable(hdev)) {
settings |= MGMT_SETTING_LE;
@@ -1344,7 +1340,7 @@ static int set_hs(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
BT_DBG("request for %s", hdev->name);
- if (!enable_hs)
+ if (!lmp_bredr_capable(hdev))
return cmd_status(sk, hdev->id, MGMT_OP_SET_HS,
MGMT_STATUS_NOT_SUPPORTED);
@@ -4396,6 +4392,3 @@ int mgmt_device_unblocked(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type)
return mgmt_event(MGMT_EV_DEVICE_UNBLOCKED, hdev, &ev, sizeof(ev),
cmd ? cmd->sk : NULL);
}
-
-module_param(enable_hs, bool, 0644);
-MODULE_PARM_DESC(enable_hs, "Enable High Speed support");
--
1.8.3.1
^ permalink raw reply related
* [PATCH 2/3] Bluetooth: Replace BDADDR_LOCAL with BDADDR_NONE
From: Marcel Holtmann @ 2013-09-30 4:11 UTC (permalink / raw)
To: linux-bluetooth
The BDADDR_LOCAL is a relict from userspace and has never been used
within the kernel. So remove that constant and replace it with a new
BDADDR_NONE that is similar to HCI_DEV_NONE with all bits set.
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Acked-by: Johan Hedberg <johan.hedberg@intel.com>
---
include/net/bluetooth/bluetooth.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index afbc711..5fd5106 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -197,8 +197,8 @@ static inline bool bdaddr_type_is_le(__u8 type)
return false;
}
-#define BDADDR_ANY (&(bdaddr_t) {{0, 0, 0, 0, 0, 0} })
-#define BDADDR_LOCAL (&(bdaddr_t) {{0, 0, 0, 0xff, 0xff, 0xff} })
+#define BDADDR_ANY (&(bdaddr_t) {{0, 0, 0, 0, 0, 0}})
+#define BDADDR_NONE (&(bdaddr_t) {{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}})
/* Copy, swap, convert BD Address */
static inline int bacmp(const bdaddr_t *ba1, const bdaddr_t *ba2)
--
1.8.3.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