Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCH 1/2] Bluetooth: Remove unneeded val variable when setting SSP
From: Anderson Lizardo @ 2013-10-10 11:00 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: BlueZ development
In-Reply-To: <1381399691-42391-1-git-send-email-marcel@holtmann.org>

Hi Marcel,

On Thu, Oct 10, 2013 at 6:08 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
> The variable val in the set_ssp() function of the management interface
> is not needed. Just use cp->val directly since its input values have
> already been validated.
> [...]
> -       if (test_bit(HCI_SSP_ENABLED, &hdev->dev_flags) == val) {
> +       if (!!cp->val == test_bit(HCI_SSP_ENABLED, &hdev->dev_flags)) {

In this case, the "!!" trick is unnecessary as the only allowed values
are 0x00 and 0x01 (I believe this has been removed in other similar
cases).

Best Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

^ permalink raw reply

* Re: [PATCH 1/2] Bluetooth: Remove unneeded val variable when setting SSP
From: Johan Hedberg @ 2013-10-10 10:56 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1381399691-42391-1-git-send-email-marcel@holtmann.org>

Hi Marcel,

On Thu, Oct 10, 2013, Marcel Holtmann wrote:
> The variable val in the set_ssp() function of the management interface
> is not needed. Just use cp->val directly since its input values have
> already been validated.
> 
> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> ---
>  net/bluetooth/mgmt.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)

Both patches have been applied to bluetooth-next. Thanks.

Johan

^ permalink raw reply

* [PATCH 2/2] Bluetooth: Restrict high speed support to SSP enabled controllers
From: Marcel Holtmann @ 2013-10-10 10:08 UTC (permalink / raw)
  To: linux-bluetooth

The support for Bluetooth High Speed can only be enabled on controllers
where also Secure Simple Pairing has been enabled. Trying to enable
high speed when SSP is disabled will result into an error. Disabling
SSP will at the same time disable high speed as well.

It is required to enforce this dependency on SSP since high speed
support is only defined for authenticated, unauthenticated and
debug link keys. These link key types require SSP.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/mgmt.c | 42 ++++++++++++++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 10 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 2fb4d35..e7ffd39 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1310,11 +1310,19 @@ static int set_ssp(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 	hci_dev_lock(hdev);
 
 	if (!hdev_is_powered(hdev)) {
-		bool changed = false;
+		bool changed;
 
-		if (val != test_bit(HCI_SSP_ENABLED, &hdev->dev_flags)) {
-			change_bit(HCI_SSP_ENABLED, &hdev->dev_flags);
-			changed = true;
+		if (cp->val) {
+			changed = !test_and_set_bit(HCI_SSP_ENABLED,
+						    &hdev->dev_flags);
+		} else {
+			changed = test_and_clear_bit(HCI_SSP_ENABLED,
+						     &hdev->dev_flags);
+			if (!changed)
+				changed = test_and_clear_bit(HCI_HS_ENABLED,
+							     &hdev->dev_flags);
+			else
+				clear_bit(HCI_HS_ENABLED, &hdev->dev_flags);
 		}
 
 		err = send_settings_rsp(sk, MGMT_OP_SET_SSP, hdev);
@@ -1327,7 +1335,8 @@ static int set_ssp(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 		goto failed;
 	}
 
-	if (mgmt_pending_find(MGMT_OP_SET_SSP, hdev)) {
+	if (mgmt_pending_find(MGMT_OP_SET_SSP, hdev) ||
+	    mgmt_pending_find(MGMT_OP_SET_HS, hdev)) {
 		err = cmd_status(sk, hdev->id, MGMT_OP_SET_SSP,
 				 MGMT_STATUS_BUSY);
 		goto failed;
@@ -1368,6 +1377,14 @@ static int set_hs(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 	if (status)
 		return cmd_status(sk, hdev->id, MGMT_OP_SET_HS, status);
 
+	if (!lmp_ssp_capable(hdev))
+		return cmd_status(sk, hdev->id, MGMT_OP_SET_HS,
+				  MGMT_STATUS_NOT_SUPPORTED);
+
+	if (!test_bit(HCI_SSP_ENABLED, &hdev->dev_flags))
+		return cmd_status(sk, hdev->id, MGMT_OP_SET_HS,
+				  MGMT_STATUS_REJECTED);
+
 	if (cp->val != 0x00 && cp->val != 0x01)
 		return cmd_status(sk, hdev->id, MGMT_OP_SET_HS,
 				  MGMT_STATUS_INVALID_PARAMS);
@@ -4403,8 +4420,10 @@ int mgmt_ssp_enable_complete(struct hci_dev *hdev, u8 enable, u8 status)
 		u8 mgmt_err = mgmt_status(status);
 
 		if (enable && test_and_clear_bit(HCI_SSP_ENABLED,
-						 &hdev->dev_flags))
+						 &hdev->dev_flags)) {
+			clear_bit(HCI_HS_ENABLED, &hdev->dev_flags);
 			err = new_settings(hdev, NULL);
+		}
 
 		mgmt_pending_foreach(MGMT_OP_SET_SSP, hdev, cmd_status_rsp,
 				     &mgmt_err);
@@ -4413,11 +4432,14 @@ int mgmt_ssp_enable_complete(struct hci_dev *hdev, u8 enable, u8 status)
 	}
 
 	if (enable) {
-		if (!test_and_set_bit(HCI_SSP_ENABLED, &hdev->dev_flags))
-			changed = true;
+		changed = !test_and_set_bit(HCI_SSP_ENABLED, &hdev->dev_flags);
 	} else {
-		if (test_and_clear_bit(HCI_SSP_ENABLED, &hdev->dev_flags))
-			changed = true;
+		changed = test_and_clear_bit(HCI_SSP_ENABLED, &hdev->dev_flags);
+		if (!changed)
+			changed = test_and_clear_bit(HCI_HS_ENABLED,
+						     &hdev->dev_flags);
+		else
+			clear_bit(HCI_HS_ENABLED, &hdev->dev_flags);
 	}
 
 	mgmt_pending_foreach(MGMT_OP_SET_SSP, hdev, settings_rsp, &match);
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH 1/2] Bluetooth: Remove unneeded val variable when setting SSP
From: Marcel Holtmann @ 2013-10-10 10:08 UTC (permalink / raw)
  To: linux-bluetooth

The variable val in the set_ssp() function of the management interface
is not needed. Just use cp->val directly since its input values have
already been validated.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/mgmt.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index a9d7506..2fb4d35 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1290,7 +1290,7 @@ static int set_ssp(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 {
 	struct mgmt_mode *cp = data;
 	struct pending_cmd *cmd;
-	u8 val, status;
+	u8 status;
 	int err;
 
 	BT_DBG("request for %s", hdev->name);
@@ -1309,8 +1309,6 @@ static int set_ssp(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 
 	hci_dev_lock(hdev);
 
-	val = !!cp->val;
-
 	if (!hdev_is_powered(hdev)) {
 		bool changed = false;
 
@@ -1335,7 +1333,7 @@ static int set_ssp(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 		goto failed;
 	}
 
-	if (test_bit(HCI_SSP_ENABLED, &hdev->dev_flags) == val) {
+	if (!!cp->val == test_bit(HCI_SSP_ENABLED, &hdev->dev_flags)) {
 		err = send_settings_rsp(sk, MGMT_OP_SET_SSP, hdev);
 		goto failed;
 	}
@@ -1346,7 +1344,7 @@ static int set_ssp(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 		goto failed;
 	}
 
-	err = hci_send_cmd(hdev, HCI_OP_WRITE_SSP_MODE, sizeof(val), &val);
+	err = hci_send_cmd(hdev, HCI_OP_WRITE_SSP_MODE, 1, &cp->val);
 	if (err < 0) {
 		mgmt_pending_remove(cmd);
 		goto failed;
-- 
1.8.3.1


^ permalink raw reply related

* Re: [PATCHv3 06/15] android: Add basic mgmt initialization sequence
From: Andrei Emeltchenko @ 2013-10-10  9:59 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <0147C8F8-0491-44CD-BCDB-51DF9F6D3315@holtmann.org>

Hi Marcel,

On Wed, Oct 09, 2013 at 09:30:22PM +0200, Marcel Holtmann wrote:
> Hi Andrei,
> 
> > Initialize bluetooth controller via mgmt interface.
> > ---
> > Makefile.android   |    4 +-
> > android/Android.mk |   11 +++
> > android/main.c     |  189 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 203 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Makefile.android b/Makefile.android
> > index e161e6d..9a2c486 100644
> > --- a/Makefile.android
> > +++ b/Makefile.android
> > @@ -1,7 +1,9 @@
> > if ANDROID
> > noinst_PROGRAMS += android/bluetoothd
> > 
> > -android_bluetoothd_SOURCES = android/main.c src/log.c
> > +android_bluetoothd_SOURCES = android/main.c src/log.c \
> > +				src/shared/util.h src/shared/util.c \
> > +				src/shared/mgmt.h src/shared/mgmt.c
> > android_bluetoothd_LDADD = @GLIB_LIBS@
> > endif
> > 
> > diff --git a/android/Android.mk b/android/Android.mk
> > index 2cabff4..f5fd863 100644
> > --- a/android/Android.mk
> > +++ b/android/Android.mk
> > @@ -15,10 +15,15 @@ include $(CLEAR_VARS)
> > LOCAL_SRC_FILES := \
> > 	main.c \
> > 	log.c \
> > +	../src/shared/mgmt.c \
> > +	../src/shared/util.c \
> > 
> > LOCAL_C_INCLUDES := \
> > 	$(call include-path-for, glib) \
> > 	$(call include-path-for, glib)/glib \
> > +
> > +LOCAL_C_INCLUDES += \
> > +	$(LOCAL_PATH)/../ \
> > 	$(LOCAL_PATH)/../src \
> do we need these nested includes actually? We could also just fix the includes. BlueZ historically has not been good with clear includes. I started to fix this, but it seems I have not gotten to all of them yet.
>

So how do you want it to be? Here I just added BlueZ top-dir.

> > LOCAL_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\"
> > @@ -26,6 +31,12 @@ LOCAL_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\"
> > # to suppress the "warning: missing initializer near initialization.." warning
> > LOCAL_CFLAGS += -Wno-missing-field-initializers
> > 
> > +# to suppress the "pointer of type 'void *' used in arithmetic" warning
> > +LOCAL_CFLAGS += -Wno-pointer-arith
> 
> Why do we need to suppress these warning. Can we not just fix them.
> 

Is this fix good:

diff --git a/src/shared/mgmt.c b/src/shared/mgmt.c
index 2c79886..b2f5506 100644
--- a/src/shared/mgmt.c
+++ b/src/shared/mgmt.c
@@ -55,7 +55,7 @@ struct mgmt {
        unsigned int next_notify_id;
        bool in_notify;
        bool destroyed;
-       void *buf;
+       uint8_t *buf;
        uint16_t len;
        mgmt_debug_func_t debug_callback;
        mgmt_destroy_func_t debug_destroy;
@@ -299,7 +299,7 @@ static gboolean received_data(GIOChannel *channel,
GIOCondition cond,
        if (bytes_read < MGMT_HDR_SIZE)
                return TRUE;
 
-       hdr = mgmt->buf;
+       hdr = (struct mgmt_hdr *) mgmt->buf;
        event = btohs(hdr->opcode);
        index = btohs(hdr->index);
        length = btohs(hdr->len);
@@ -309,7 +309,7 @@ static gboolean received_data(GIOChannel *channel,
GIOCondition cond,
 
        switch (event) {
        case MGMT_EV_CMD_COMPLETE:
-               cc = mgmt->buf + MGMT_HDR_SIZE;
+               cc = (struct mgmt_ev_cmd_complete *) mgmt->buf +
MGMT_HDR_SIZE;
                opcode = btohs(cc->opcode);
 
                util_debug(mgmt->debug_callback, mgmt->debug_data,
@@ -320,7 +320,7 @@ static gboolean received_data(GIOChannel *channel,
GIOCondition cond,
                                                mgmt->buf + MGMT_HDR_SIZE
+ 3);
                break;
        case MGMT_EV_CMD_STATUS:
-               cs = mgmt->buf + MGMT_HDR_SIZE;
+               cs = (struct mgmt_ev_cmd_status *) mgmt->buf +
MGMT_HDR_SIZE;
                opcode = btohs(cs->opcode);
 
                util_debug(mgmt->debug_callback, mgmt->debug_data,

> > +
> > +# Define missing flags for Android 4.2
> > +LOCAL_CFLAGS += -DSOCK_CLOEXEC=02000000 -DSOCK_NONBLOCK=04000
> > +
> 
> This thing is dangerous. Do we really bother with Android 4.2 support
> and can not rely on a newer version that has this fixed properly in
> bionic.

The problem here is that our base android-ia from 01.org is based on
4.2.2. Do you still want to remove this?

Best regards 
Andrei Emeltchenko 

^ permalink raw reply related

* [PATCH v2] Don't register Device ID record on sdp server start
From: Szymon Janc @ 2013-10-10  9:38 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1381397344-7389-1-git-send-email-szymon.janc@tieto.com>

This makes SDP code no longer depends on main_opts. DID record is now
registered from main() after sdp server was started. This is OK since
mainloop is not yet running and record will be present when first
request comes.
---

v2 - commit message improved

 src/main.c        | 4 ++++
 src/sdpd-server.c | 5 -----
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/main.c b/src/main.c
index eafe2ed..91d90b4 100644
--- a/src/main.c
+++ b/src/main.c
@@ -550,6 +550,10 @@ int main(int argc, char *argv[])
 
 	start_sdp_server(sdp_mtu, sdp_flags);
 
+	if (main_opts.did_source > 0)
+		register_device_id(main_opts.did_source, main_opts.did_vendor,
+				main_opts.did_product, main_opts.did_version);
+
 	/* Loading plugins has to be done after D-Bus has been setup since
 	 * the plugins might wanna expose some paths on the bus. However the
 	 * best order of how to init various subsystems of the Bluetooth
diff --git a/src/sdpd-server.c b/src/sdpd-server.c
index 10e46a1..b411abe 100644
--- a/src/sdpd-server.c
+++ b/src/sdpd-server.c
@@ -42,7 +42,6 @@
 
 #include <glib.h>
 
-#include "hcid.h"
 #include "log.h"
 #include "sdpd.h"
 
@@ -238,10 +237,6 @@ int start_sdp_server(uint16_t mtu, uint32_t flags)
 		return -1;
 	}
 
-	if (main_opts.did_source > 0)
-		register_device_id(main_opts.did_source, main_opts.did_vendor,
-				main_opts.did_product, main_opts.did_version);
-
 	io = g_io_channel_unix_new(l2cap_sock);
 	g_io_channel_set_close_on_unref(io, TRUE);
 
-- 
1.8.4


^ permalink raw reply related

* [PATCH] Don't register Device ID record on sdp server start
From: Szymon Janc @ 2013-10-10  9:29 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

DID record is now registered from main() after sdp server was started.
This is OK since mainloop is not yet running and record will be present
when first request comes.

Another benefit is that sdp code no longer depends on main_opts.
---
 src/main.c        | 4 ++++
 src/sdpd-server.c | 5 -----
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/main.c b/src/main.c
index eafe2ed..91d90b4 100644
--- a/src/main.c
+++ b/src/main.c
@@ -550,6 +550,10 @@ int main(int argc, char *argv[])
 
 	start_sdp_server(sdp_mtu, sdp_flags);
 
+	if (main_opts.did_source > 0)
+		register_device_id(main_opts.did_source, main_opts.did_vendor,
+				main_opts.did_product, main_opts.did_version);
+
 	/* Loading plugins has to be done after D-Bus has been setup since
 	 * the plugins might wanna expose some paths on the bus. However the
 	 * best order of how to init various subsystems of the Bluetooth
diff --git a/src/sdpd-server.c b/src/sdpd-server.c
index 10e46a1..b411abe 100644
--- a/src/sdpd-server.c
+++ b/src/sdpd-server.c
@@ -42,7 +42,6 @@
 
 #include <glib.h>
 
-#include "hcid.h"
 #include "log.h"
 #include "sdpd.h"
 
@@ -238,10 +237,6 @@ int start_sdp_server(uint16_t mtu, uint32_t flags)
 		return -1;
 	}
 
-	if (main_opts.did_source > 0)
-		register_device_id(main_opts.did_source, main_opts.did_vendor,
-				main_opts.did_product, main_opts.did_version);
-
 	io = g_io_channel_unix_new(l2cap_sock);
 	g_io_channel_set_close_on_unref(io, TRUE);
 
-- 
1.8.4


^ permalink raw reply related

* Re: [PATCHv3 08/15] android: Add adapter and device struct for BlueZ daemon
From: Marcin Kraglak @ 2013-10-10  9:07 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1381243883-2745-9-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

On 8 October 2013 16:51, Andrei Emeltchenko
<Andrei.Emeltchenko.news@gmail.com> wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> Adapter structure in BlueZ daemon keeps track of default adapter
> and device structure keeps track about found devices.
> ---
>  android/bt_adapter.c |   56 ++++++++++++++++++++++++++++++++++++++++++++++
>  android/bt_adapter.h |   60 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 116 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..e21d50c
> --- /dev/null
> +++ b/android/bt_adapter.c
> @@ -0,0 +1,56 @@
> +/*
> + *
> + *  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_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 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..6877cc7
> --- /dev/null
> +++ b/android/bt_adapter.h
> @@ -0,0 +1,60 @@
> +/*
> + *
> + *  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;
> +};

I'm not sure if we need struct bt_device. Shouldn't we just use bdaddr_t?
All data from this struct will be cached in upper layers, so we don't need to
store it in daemon.

> +
> +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;
> +};
> +
> +struct bt_adapter *bt_adapter_new(uint16_t index, struct mgmt *mgmt_if);
> +
> +void adapter_start(struct bt_adapter *adapter);
> +void adapter_stop(struct bt_adapter *adapter);
> --
> 1.7.10.4
>
> --
> 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

Best regards
Marcin

^ permalink raw reply

* [PATCH 6/6] avdtp: Remove not needed forward declaration
From: Szymon Janc @ 2013-10-10  8:53 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1381395221-5067-1-git-send-email-szymon.janc@tieto.com>

auth_cb function is not used before definition so no need to forward
declare it.
---
 profiles/audio/avdtp.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index e5d4139..5b8d7b1 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -442,7 +442,6 @@ static int process_queue(struct avdtp *session);
 static void avdtp_sep_set_state(struct avdtp *session,
 				struct avdtp_local_sep *sep,
 				avdtp_state_t state);
-static void auth_cb(DBusError *derr, void *user_data);
 
 static struct avdtp_server *find_server(GSList *list, struct btd_adapter *a)
 {
-- 
1.8.4


^ permalink raw reply related

* [PATCH 5/6] avdtp: Move connection_lost function to avoid forward declaration
From: Szymon Janc @ 2013-10-10  8:53 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1381395221-5067-1-git-send-email-szymon.janc@tieto.com>

---
 profiles/audio/avdtp.c | 53 +++++++++++++++++++++++++-------------------------
 1 file changed, 26 insertions(+), 27 deletions(-)

diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index bf15935..e5d4139 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -439,7 +439,6 @@ static gboolean avdtp_parse_rej(struct avdtp *session,
 					uint8_t transaction, uint8_t signal_id,
 					void *buf, int size);
 static int process_queue(struct avdtp *session);
-static void connection_lost(struct avdtp *session, int err);
 static void avdtp_sep_set_state(struct avdtp *session,
 				struct avdtp_local_sep *sep,
 				avdtp_state_t state);
@@ -1137,6 +1136,32 @@ static void avdtp_free(void *data)
 	g_free(session);
 }
 
+static void connection_lost(struct avdtp *session, int err)
+{
+	struct avdtp_server *server = session->server;
+	char address[18];
+
+	ba2str(device_get_address(session->device), address);
+	DBG("Disconnected from %s", address);
+
+	if (err != EACCES)
+		avdtp_cancel_authorization(session);
+
+	g_slist_foreach(session->streams, (GFunc) release_stream, session);
+	session->streams = NULL;
+
+	finalize_discovery(session, err);
+
+	avdtp_set_state(session, AVDTP_SESSION_STATE_DISCONNECTED);
+
+	if (session->ref > 0)
+		return;
+
+	server->sessions = g_slist_remove(server->sessions, session);
+	btd_device_unref(session->device);
+	avdtp_free(session);
+}
+
 static gboolean disconnect_timeout(gpointer user_data)
 {
 	struct avdtp *session = user_data;
@@ -1180,32 +1205,6 @@ static void set_disconnect_timer(struct avdtp *session)
 						session);
 }
 
-static void connection_lost(struct avdtp *session, int err)
-{
-	struct avdtp_server *server = session->server;
-	char address[18];
-
-	ba2str(device_get_address(session->device), address);
-	DBG("Disconnected from %s", address);
-
-	if (err != EACCES)
-		avdtp_cancel_authorization(session);
-
-	g_slist_foreach(session->streams, (GFunc) release_stream, session);
-	session->streams = NULL;
-
-	finalize_discovery(session, err);
-
-	avdtp_set_state(session, AVDTP_SESSION_STATE_DISCONNECTED);
-
-	if (session->ref > 0)
-		return;
-
-	server->sessions = g_slist_remove(server->sessions, session);
-	btd_device_unref(session->device);
-	avdtp_free(session);
-}
-
 void avdtp_unref(struct avdtp *session)
 {
 	if (!session)
-- 
1.8.4


^ permalink raw reply related

* [PATCH 4/6] avdtp: Rename avdtp_callbacks to state_callbacks
From: Szymon Janc @ 2013-10-10  8:53 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1381395221-5067-1-git-send-email-szymon.janc@tieto.com>

This better describes what list contains.
---
 profiles/audio/avdtp.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index e83245e..bf15935 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -425,7 +425,7 @@ struct avdtp {
 
 static GSList *servers = NULL;
 
-static GSList *avdtp_callbacks = NULL;
+static GSList *state_callbacks = NULL;
 
 static int send_request(struct avdtp *session, gboolean priority,
 			struct avdtp_stream *stream, uint8_t signal_id,
@@ -710,7 +710,7 @@ static void avdtp_set_state(struct avdtp *session,
 
 	session->state = new_state;
 
-	for (l = avdtp_callbacks; l != NULL; l = l->next) {
+	for (l = state_callbacks; l != NULL; l = l->next) {
 		struct avdtp_state_callback *cb = l->data;
 
 		if (session->device != cb->dev)
@@ -3874,7 +3874,8 @@ unsigned int avdtp_add_state_cb(struct btd_device *dev,
 	state_cb->id = ++id;
 	state_cb->user_data = user_data;
 
-	avdtp_callbacks = g_slist_append(avdtp_callbacks, state_cb);
+	state_callbacks = g_slist_append(state_callbacks,
+								state_cb);
 
 	return state_cb->id;
 }
@@ -3883,10 +3884,10 @@ gboolean avdtp_remove_state_cb(unsigned int id)
 {
 	GSList *l;
 
-	for (l = avdtp_callbacks; l != NULL; l = l->next) {
+	for (l = state_callbacks; l != NULL; l = l->next) {
 		struct avdtp_state_callback *cb = l->data;
 		if (cb && cb->id == id) {
-			avdtp_callbacks = g_slist_remove(avdtp_callbacks, cb);
+			state_callbacks = g_slist_remove(state_callbacks, cb);
 			g_free(cb);
 			return TRUE;
 		}
-- 
1.8.4


^ permalink raw reply related

* [PATCH 3/6] avdtp: Remove unused avdtp_stream_setup_active function
From: Szymon Janc @ 2013-10-10  8:53 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1381395221-5067-1-git-send-email-szymon.janc@tieto.com>

It is not used anywhere and can be removed.
---
 profiles/audio/avdtp.c | 5 -----
 profiles/audio/avdtp.h | 1 -
 2 files changed, 6 deletions(-)

diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index 1f29050..e83245e 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -3857,11 +3857,6 @@ gboolean avdtp_has_stream(struct avdtp *session, struct avdtp_stream *stream)
 	return g_slist_find(session->streams, stream) ? TRUE : FALSE;
 }
 
-gboolean avdtp_stream_setup_active(struct avdtp *session)
-{
-	return session->stream_setup;
-}
-
 void avdtp_set_device_disconnect(struct avdtp *session, gboolean dev_dc)
 {
 	session->device_disconnect = dev_dc;
diff --git a/profiles/audio/avdtp.h b/profiles/audio/avdtp.h
index e19dfc3..3bf7503 100644
--- a/profiles/audio/avdtp.h
+++ b/profiles/audio/avdtp.h
@@ -307,7 +307,6 @@ int avdtp_error_posix_errno(struct avdtp_error *err);
 struct btd_adapter *avdtp_get_adapter(struct avdtp *session);
 struct btd_device *avdtp_get_device(struct avdtp *session);
 
-gboolean avdtp_stream_setup_active(struct avdtp *session);
 void avdtp_set_device_disconnect(struct avdtp *session, gboolean dev_dc);
 
 int avdtp_init(struct btd_adapter *adapter);
-- 
1.8.4


^ permalink raw reply related

* [PATCH 2/6] avdtp: Remove unused avdtp_is_connected function
From: Szymon Janc @ 2013-10-10  8:53 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1381395221-5067-1-git-send-email-szymon.janc@tieto.com>

It is not used anywhere and can be removed.
---
 profiles/audio/avdtp.c | 19 -------------------
 profiles/audio/avdtp.h |  2 --
 2 files changed, 21 deletions(-)

diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index fc896c7..1f29050 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -3164,25 +3164,6 @@ static gboolean avdtp_parse_rej(struct avdtp *session,
 	}
 }
 
-gboolean avdtp_is_connected(struct btd_device *device)
-{
-	struct avdtp_server *server;
-	struct avdtp *session;
-
-	server = find_server(servers, device_get_adapter(device));
-	if (!server)
-		return FALSE;
-
-	session = find_session(server->sessions, device);
-	if (!session)
-		return FALSE;
-
-	if (session->state != AVDTP_SESSION_STATE_DISCONNECTED)
-		return TRUE;
-
-	return FALSE;
-}
-
 struct avdtp_service_capability *avdtp_stream_get_codec(
 						struct avdtp_stream *stream)
 {
diff --git a/profiles/audio/avdtp.h b/profiles/audio/avdtp.h
index b8aaf1d..e19dfc3 100644
--- a/profiles/audio/avdtp.h
+++ b/profiles/audio/avdtp.h
@@ -218,8 +218,6 @@ struct avdtp *avdtp_get(struct btd_device *device);
 void avdtp_unref(struct avdtp *session);
 struct avdtp *avdtp_ref(struct avdtp *session);
 
-gboolean avdtp_is_connected(struct btd_device *device);
-
 struct avdtp_service_capability *avdtp_service_cap_new(uint8_t category,
 							void *data, int size);
 
-- 
1.8.4


^ permalink raw reply related

* [PATCH 1/6] avdtp: Fix typos in errors from avdtp_strerror
From: Szymon Janc @ 2013-10-10  8:53 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

---
 profiles/audio/avdtp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index fe63a40..fc896c7 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -3807,7 +3807,7 @@ const char *avdtp_strerror(struct avdtp_error *err)
 	case AVDTP_BAD_ROHC_FORMAT:
 		return "Bad Header Compression Format";
 	case AVDTP_BAD_CP_FORMAT:
-		return "Bad Content Protetion Format";
+		return "Bad Content Protection Format";
 	case AVDTP_BAD_MULTIPLEXING_FORMAT:
 		return "Bad Multiplexing Format";
 	case AVDTP_UNSUPPORTED_CONFIGURATION:
@@ -3815,7 +3815,7 @@ const char *avdtp_strerror(struct avdtp_error *err)
 	case AVDTP_BAD_STATE:
 		return "Bad State";
 	default:
-		return "Unknow error";
+		return "Unknown error";
 	}
 }
 
-- 
1.8.4


^ permalink raw reply related

* Re: [PATCH v4 2/2] Bluetooth: Refactor hci_connect_le
From: Marcel Holtmann @ 2013-10-10  8:31 UTC (permalink / raw)
  To: Andre Guedes; +Cc: linux-bluetooth
In-Reply-To: <1381231278-20534-2-git-send-email-andre.guedes@openbossa.org>

Hi Andrei,

> This patch does some code refactoring in hci_connect_le() by moving
> the exception code into if statements and letting the main flow in
> first level of function scope. It also adds extra comments to improve
> the code readability.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> net/bluetooth/hci_conn.c | 51 +++++++++++++++++++++++++++++++-----------------
> 1 file changed, 33 insertions(+), 18 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH v4 1/2] Bluetooth: Use HCI request for LE connection
From: Marcel Holtmann @ 2013-10-10  8:31 UTC (permalink / raw)
  To: Andre Guedes; +Cc: linux-bluetooth
In-Reply-To: <1381231278-20534-1-git-send-email-andre.guedes@openbossa.org>

Hi Andre,

> This patch introduces a new helper, which uses the HCI request
> framework, for creating LE connectons. All the handling is now
> done by this function so we can remove the hci_cs_le_create_conn()
> event handler.
> 
> This patch also removes the old hci_le_create_connection() since
> it is not used anymore.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> net/bluetooth/hci_conn.c  | 91 ++++++++++++++++++++++++++++++++++-------------
> net/bluetooth/hci_event.c | 31 ----------------
> 2 files changed, 67 insertions(+), 55 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel


^ permalink raw reply

* Re: [PATCHv3 01/15] android: Supress missing initializers warnings
From: Andrei Emeltchenko @ 2013-10-10  8:16 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <982246CD-1B58-457A-85C3-4EA584226E3C@holtmann.org>

Hi Marcel,

On Thu, Oct 10, 2013 at 10:10:37AM +0200, Marcel Holtmann wrote:
> Hi Andrei,
> 
> >>>>> These types of warnings are disabled in BlueZ makeifiles
> >>>>> 
> >>>>> main.c:67:2: warning: missing initializer [-Wmissing-field-initializers]
> >>>>> main.c:67:2: warning: (near initialization for 'options[1].short_name')
> >>>>> 	[-Wmissing-field-initializers]
> >>>>> ---
> >>>>> android/Android.mk |    3 +++
> >>>>> 1 file changed, 3 insertions(+)
> >>>>> 
> >>>>> diff --git a/android/Android.mk b/android/Android.mk
> >>>>> index ec820ac..5498b41 100644
> >>>>> --- a/android/Android.mk
> >>>>> +++ b/android/Android.mk
> >>>>> @@ -23,6 +23,9 @@ LOCAL_C_INCLUDES := \
> >>>>> 
> >>>>> LOCAL_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\"
> >>>>> 
> >>>>> +# to suppress the "warning: missing initializer near initialization.." warning
> >>>>> +LOCAL_CFLAGS += -Wno-missing-field-initializers
> >>>>> +
> >>>> 
> >>>> why are we doing this and not fixing the actual warning?
> >>>> 
> >>> 
> >>> Do you want me to initialize structures like
> >>> 
> >>> {NULL, NULL, NULL, ... }
> >>> 
> >>> this way it can be fixed.
> >> 
> >> that is ugly and gets really complicated in the code over time. So I prefer we do not do that.
> >> 
> >> Which structures does this affect. All of them?
> > 
> > You can just try following patch and see:
> > 
> > diff --git a/acinclude.m4 b/acinclude.m4
> > index 5bfa29d..4d6a42c 100644
> > --- a/acinclude.m4
> > +++ b/acinclude.m4
> > @@ -15,7 +15,6 @@ AC_DEFUN([COMPILER_FLAGS], [
> >        if (test "$USE_MAINTAINER_MODE" = "yes"); then
> >                with_cflags="$with_cflags -Wall -Werror -Wextra"
> >                with_cflags="$with_cflags -Wno-unused-parameter"
> > -               with_cflags="$with_cflags -Wno-missing-field-initializers"
> >                with_cflags="$with_cflags -Wdeclaration-after-statement"
> >                with_cflags="$with_cflags -Wmissing-declarations"
> >                with_cflags="$with_cflags -Wredundant-decls"
> 
> can we then have a global variable in Android.mk that lists all of our
> warning exceptions. Re-assigning them over and over again manually seems
> a rather bad idea.

We can create ANDROID_CFLAGS and include it to other targets. Is this
good?

Best regards 
Andrei Emeltchenko 

^ permalink raw reply

* [PATCH] emulator: Fix AMP emulation build error
From: Szymon Janc @ 2013-10-10  8:15 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

This fix build with GCC 4.6.3.

  CC     emulator/amp.o
emulator/amp.c: In function ‘send_packet’:
emulator/amp.c:172:7: error: ignoring return value of ‘write’,
    declared with attribute warn_unused_result [-Werror=unused-result]
cc1: all warnings being treated as errors
make[1]: *** [emulator/amp.o] Error
---
 emulator/amp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/emulator/amp.c b/emulator/amp.c
index e80d9a8..714854b 100644
--- a/emulator/amp.c
+++ b/emulator/amp.c
@@ -169,7 +169,8 @@ static void reset_defaults(struct bt_amp *amp)
 
 static void send_packet(struct bt_amp *amp, const void *data, uint16_t len)
 {
-	write(amp->vhci_fd, data, len);
+	if (write(amp->vhci_fd, data, len) < 0)
+		fprintf(stderr, "Write to /dev/vhci failed\n");
 }
 
 static void send_event(struct bt_amp *amp, uint8_t event,
-- 
1.8.4


^ permalink raw reply related

* Re: [PATCHv3 01/15] android: Supress missing initializers warnings
From: Marcel Holtmann @ 2013-10-10  8:10 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <20131010080657.GF23879@aemeltch-MOBL1>

Hi Andrei,

>>>>> These types of warnings are disabled in BlueZ makeifiles
>>>>> 
>>>>> main.c:67:2: warning: missing initializer [-Wmissing-field-initializers]
>>>>> main.c:67:2: warning: (near initialization for 'options[1].short_name')
>>>>> 	[-Wmissing-field-initializers]
>>>>> ---
>>>>> android/Android.mk |    3 +++
>>>>> 1 file changed, 3 insertions(+)
>>>>> 
>>>>> diff --git a/android/Android.mk b/android/Android.mk
>>>>> index ec820ac..5498b41 100644
>>>>> --- a/android/Android.mk
>>>>> +++ b/android/Android.mk
>>>>> @@ -23,6 +23,9 @@ LOCAL_C_INCLUDES := \
>>>>> 
>>>>> LOCAL_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\"
>>>>> 
>>>>> +# to suppress the "warning: missing initializer near initialization.." warning
>>>>> +LOCAL_CFLAGS += -Wno-missing-field-initializers
>>>>> +
>>>> 
>>>> why are we doing this and not fixing the actual warning?
>>>> 
>>> 
>>> Do you want me to initialize structures like
>>> 
>>> {NULL, NULL, NULL, ... }
>>> 
>>> this way it can be fixed.
>> 
>> that is ugly and gets really complicated in the code over time. So I prefer we do not do that.
>> 
>> Which structures does this affect. All of them?
> 
> You can just try following patch and see:
> 
> diff --git a/acinclude.m4 b/acinclude.m4
> index 5bfa29d..4d6a42c 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -15,7 +15,6 @@ AC_DEFUN([COMPILER_FLAGS], [
>        if (test "$USE_MAINTAINER_MODE" = "yes"); then
>                with_cflags="$with_cflags -Wall -Werror -Wextra"
>                with_cflags="$with_cflags -Wno-unused-parameter"
> -               with_cflags="$with_cflags -Wno-missing-field-initializers"
>                with_cflags="$with_cflags -Wdeclaration-after-statement"
>                with_cflags="$with_cflags -Wmissing-declarations"
>                with_cflags="$with_cflags -Wredundant-decls"

can we then have a global variable in Android.mk that lists all of our warning exceptions. Re-assigning them over and over again manually seems a rather bad idea.

Regards

Marcel


^ permalink raw reply

* Re: [PATCHv3 01/15] android: Supress missing initializers warnings
From: Andrei Emeltchenko @ 2013-10-10  8:07 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <7EFBFA1F-E02A-454D-B93E-1252D4288C87@holtmann.org>

Hi Marcel,

On Thu, Oct 10, 2013 at 09:35:40AM +0200, Marcel Holtmann wrote:
> Hi Andrei,
> 
> >>> These types of warnings are disabled in BlueZ makeifiles
> >>> 
> >>> main.c:67:2: warning: missing initializer [-Wmissing-field-initializers]
> >>> main.c:67:2: warning: (near initialization for 'options[1].short_name')
> >>> 	[-Wmissing-field-initializers]
> >>> ---
> >>> android/Android.mk |    3 +++
> >>> 1 file changed, 3 insertions(+)
> >>> 
> >>> diff --git a/android/Android.mk b/android/Android.mk
> >>> index ec820ac..5498b41 100644
> >>> --- a/android/Android.mk
> >>> +++ b/android/Android.mk
> >>> @@ -23,6 +23,9 @@ LOCAL_C_INCLUDES := \
> >>> 
> >>> LOCAL_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\"
> >>> 
> >>> +# to suppress the "warning: missing initializer near initialization.." warning
> >>> +LOCAL_CFLAGS += -Wno-missing-field-initializers
> >>> +
> >> 
> >> why are we doing this and not fixing the actual warning?
> >> 
> > 
> > Do you want me to initialize structures like
> > 
> > {NULL, NULL, NULL, ... }
> > 
> > this way it can be fixed.
> 
> that is ugly and gets really complicated in the code over time. So I prefer we do not do that.
> 
> Which structures does this affect. All of them?

You can just try following patch and see:

diff --git a/acinclude.m4 b/acinclude.m4
index 5bfa29d..4d6a42c 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -15,7 +15,6 @@ AC_DEFUN([COMPILER_FLAGS], [
        if (test "$USE_MAINTAINER_MODE" = "yes"); then
                with_cflags="$with_cflags -Wall -Werror -Wextra"
                with_cflags="$with_cflags -Wno-unused-parameter"
-               with_cflags="$with_cflags -Wno-missing-field-initializers"
                with_cflags="$with_cflags -Wdeclaration-after-statement"
                with_cflags="$with_cflags -Wmissing-declarations"
                with_cflags="$with_cflags -Wredundant-decls"


Best regards 
Andrei Emeltchenko 

^ permalink raw reply related

* Re: [PATCHv3 01/15] android: Supress missing initializers warnings
From: Marcel Holtmann @ 2013-10-10  7:35 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <20131010065825.GC23879@aemeltch-MOBL1>

Hi Andrei,

>>> These types of warnings are disabled in BlueZ makeifiles
>>> 
>>> main.c:67:2: warning: missing initializer [-Wmissing-field-initializers]
>>> main.c:67:2: warning: (near initialization for 'options[1].short_name')
>>> 	[-Wmissing-field-initializers]
>>> ---
>>> android/Android.mk |    3 +++
>>> 1 file changed, 3 insertions(+)
>>> 
>>> diff --git a/android/Android.mk b/android/Android.mk
>>> index ec820ac..5498b41 100644
>>> --- a/android/Android.mk
>>> +++ b/android/Android.mk
>>> @@ -23,6 +23,9 @@ LOCAL_C_INCLUDES := \
>>> 
>>> LOCAL_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\"
>>> 
>>> +# to suppress the "warning: missing initializer near initialization.." warning
>>> +LOCAL_CFLAGS += -Wno-missing-field-initializers
>>> +
>> 
>> why are we doing this and not fixing the actual warning?
>> 
> 
> Do you want me to initialize structures like
> 
> {NULL, NULL, NULL, ... }
> 
> this way it can be fixed.

that is ugly and gets really complicated in the code over time. So I prefer we do not do that.

Which structures does this affect. All of them?

Regards

Marcel



^ permalink raw reply

* Re: [PATCHv3 03/15] android: Add Socket Bluetooth HAL template
From: Marcel Holtmann @ 2013-10-10  7:33 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <20131010065631.GB23879@aemeltch-MOBL1>

Hi Andrei,

>>> +static bt_status_t listen(btsock_type_t type, const char *service_name,
>>> +					const uint8_t *uuid, int chan,
>>> +					int *sock, int flags)
>>> +{
>>> +	if ((uuid == NULL && chan <= 0) || sock == NULL) {
>> 
>> we are moving away from uuid == NULL checks. Use !uuid instead.
> 
> I do not like myself these kind of checks but since this seems to be BlueZ
> style. Should I change comparison val != NULL and val != 0 to !val ? Like
> in kernel?

yes, we are slowly moving towards val and !val checks. Especially for new code that coding style has to be enforced now.

Regards

Marcel


^ permalink raw reply

* Re: [PATCHv3 01/15] android: Supress missing initializers warnings
From: Andrei Emeltchenko @ 2013-10-10  6:58 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <E4E245F7-8EE6-4FE2-AC99-3246F1541C99@holtmann.org>

Hi Marcel,

On Wed, Oct 09, 2013 at 09:11:04PM +0200, Marcel Holtmann wrote:
> Hi Andrei,
> 
> > These types of warnings are disabled in BlueZ makeifiles
> > 
> > main.c:67:2: warning: missing initializer [-Wmissing-field-initializers]
> > main.c:67:2: warning: (near initialization for 'options[1].short_name')
> > 	[-Wmissing-field-initializers]
> > ---
> > android/Android.mk |    3 +++
> > 1 file changed, 3 insertions(+)
> > 
> > diff --git a/android/Android.mk b/android/Android.mk
> > index ec820ac..5498b41 100644
> > --- a/android/Android.mk
> > +++ b/android/Android.mk
> > @@ -23,6 +23,9 @@ LOCAL_C_INCLUDES := \
> > 
> > LOCAL_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\"
> > 
> > +# to suppress the "warning: missing initializer near initialization.." warning
> > +LOCAL_CFLAGS += -Wno-missing-field-initializers
> > +
> 
> why are we doing this and not fixing the actual warning?
> 

Do you want me to initialize structures like

{NULL, NULL, NULL, ... }

this way it can be fixed.

Best regards 
Andrei Emeltchenko 

^ permalink raw reply

* Re: [PATCHv3 03/15] android: Add Socket Bluetooth HAL template
From: Andrei Emeltchenko @ 2013-10-10  6:56 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <CCFB8A73-18EC-4FE3-9C92-8FBD91B9F6B9@holtmann.org>

Hi Marcel,

On Wed, Oct 09, 2013 at 09:14:05PM +0200, Marcel Holtmann wrote:
> > +static bt_status_t listen(btsock_type_t type, const char *service_name,
> > +					const uint8_t *uuid, int chan,
> > +					int *sock, int flags)
> > +{
> > +	if ((uuid == NULL && chan <= 0) || sock == NULL) {
> 
> we are moving away from uuid == NULL checks. Use !uuid instead.

I do not like myself these kind of checks but since this seems to be BlueZ
style. Should I change comparison val != NULL and val != 0 to !val ? Like
in kernel?

Best regards 
Andrei Emeltchenko 


^ permalink raw reply

* Re: [PATCHv3 01/15] android: Supress missing initializers warnings
From: Lucas De Marchi @ 2013-10-09 22:55 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Andrei Emeltchenko, BlueZ development
In-Reply-To: <E4E245F7-8EE6-4FE2-AC99-3246F1541C99@holtmann.org>

On Wed, Oct 9, 2013 at 4:11 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Andrei,
>
>> These types of warnings are disabled in BlueZ makeifiles
>>
>> main.c:67:2: warning: missing initializer [-Wmissing-field-initializers]
>> main.c:67:2: warning: (near initialization for 'options[1].short_name')
>>       [-Wmissing-field-initializers]
>> ---
>> android/Android.mk |    3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/android/Android.mk b/android/Android.mk
>> index ec820ac..5498b41 100644
>> --- a/android/Android.mk
>> +++ b/android/Android.mk
>> @@ -23,6 +23,9 @@ LOCAL_C_INCLUDES := \
>>
>> LOCAL_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\"
>>
>> +# to suppress the "warning: missing initializer near initialization.." warning
>> +LOCAL_CFLAGS += -Wno-missing-field-initializers
>> +
>
> why are we doing this and not fixing the actual warning?

I guess because this is a stupid warning.

We do the same in autotools because otherwise we can't initialize
structs and rely on compiler setting the rest to 0. This is
particularly annoying on d-bus tables (see the exists and flags on the
end of the GDBusPropertyTable struct). I'm sure it's annoying in other
places, too.


Lucas De Marchi

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox