diff mbox series

[v1,1/5] avdtp: Add a flag in struct avdtp to control a2dp offload

Message ID 20211115094108.24331-1-kiran.k@intel.com (mailing list archive)
State Changes Requested
Headers show
Series [v1,1/5] avdtp: Add a flag in struct avdtp to control a2dp offload | expand

Checks

Context Check Description
tedd_an/checkpatch success Checkpatch PASS
tedd_an/gitlint success Gitlint PASS
tedd_an/setupell success Setup ELL PASS
tedd_an/buildprep success Build Prep PASS
tedd_an/build success Build Configuration PASS
tedd_an/makecheck fail Make Check FAIL: profiles/audio/avdtp.c: In function ‘avdtp_connect_cb’: profiles/audio/avdtp.c:2374:6: error: ‘cap’ may be used uninitialized in this function [-Werror=maybe-uninitialized] 2374 | if (setsockopt(sock, SOL_BLUETOOTH, BT_MSFT_OPEN, cap, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 2375 | sizeof(*cap) + cap->length)) | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ profiles/audio/avdtp.c:2358:35: note: ‘cap’ was declared here 2358 | struct avdtp_service_capability *cap; | ^~~ cc1: all warnings being treated as errors make[1]: *** [Makefile:8640: profiles/audio/bluetoothd-avdtp.o] Error 1 make: *** [Makefile:10501: check] Error 2
tedd_an/makedistcheck success Make Distcheck PASS
tedd_an/build_extell success Build External ELL PASS
tedd_an/build_extell_make fail Build Make with External ELL FAIL: profiles/audio/avdtp.c: In function ‘avdtp_connect_cb’: profiles/audio/avdtp.c:2374:6: error: ‘cap’ may be used uninitialized in this function [-Werror=maybe-uninitialized] 2374 | if (setsockopt(sock, SOL_BLUETOOTH, BT_MSFT_OPEN, cap, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 2375 | sizeof(*cap) + cap->length)) | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ profiles/audio/avdtp.c:2358:35: note: ‘cap’ was declared here 2358 | struct avdtp_service_capability *cap; | ^~~ cc1: all warnings being treated as errors make[1]: *** [Makefile:8640: profiles/audio/bluetoothd-avdtp.o] Error 1 make: *** [Makefile:4175: all] Error 2

Commit Message

K, Kiran Nov. 15, 2021, 9:41 a.m. UTC 
Define a flag in struct avdtp and set it based on
the definition of env variable USE_OFFLOAD
---
 profiles/audio/avdtp.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

bluez.test.bot@gmail.com Nov. 15, 2021, 9:55 a.m. UTC | #1 
This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=580019

---Test result---

Test Summary:
CheckPatch                    PASS      7.21 seconds
GitLint                       PASS      4.81 seconds
Prep - Setup ELL              PASS      42.19 seconds
Build - Prep                  PASS      0.53 seconds
Build - Configure             PASS      7.91 seconds
Build - Make                  FAIL      138.55 seconds
Make Check                    FAIL      1.60 seconds
Make Distcheck                PASS      214.63 seconds
Build w/ext ELL - Configure   PASS      8.04 seconds
Build w/ext ELL - Make        FAIL      128.87 seconds

Details
##############################
Test: Build - Make - FAIL
Desc: Build the BlueZ source tree
Output:
profiles/audio/avdtp.c: In function ‘avdtp_connect_cb’:
profiles/audio/avdtp.c:2374:6: error: ‘cap’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
 2374 |  if (setsockopt(sock, SOL_BLUETOOTH, BT_MSFT_OPEN, cap,
      |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 2375 |          sizeof(*cap) + cap->length))
      |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~
profiles/audio/avdtp.c:2358:35: note: ‘cap’ was declared here
 2358 |  struct avdtp_service_capability *cap;
      |                                   ^~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:8640: profiles/audio/bluetoothd-avdtp.o] Error 1
make: *** [Makefile:4175: all] Error 2


##############################
Test: Make Check - FAIL
Desc: Run 'make check'
Output:
profiles/audio/avdtp.c: In function ‘avdtp_connect_cb’:
profiles/audio/avdtp.c:2374:6: error: ‘cap’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
 2374 |  if (setsockopt(sock, SOL_BLUETOOTH, BT_MSFT_OPEN, cap,
      |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 2375 |          sizeof(*cap) + cap->length))
      |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~
profiles/audio/avdtp.c:2358:35: note: ‘cap’ was declared here
 2358 |  struct avdtp_service_capability *cap;
      |                                   ^~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:8640: profiles/audio/bluetoothd-avdtp.o] Error 1
make: *** [Makefile:10501: check] Error 2


##############################
Test: Build w/ext ELL - Make - FAIL
Desc: Build BlueZ source with '--enable-external-ell' configuration
Output:
profiles/audio/avdtp.c: In function ‘avdtp_connect_cb’:
profiles/audio/avdtp.c:2374:6: error: ‘cap’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
 2374 |  if (setsockopt(sock, SOL_BLUETOOTH, BT_MSFT_OPEN, cap,
      |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 2375 |          sizeof(*cap) + cap->length))
      |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~
profiles/audio/avdtp.c:2358:35: note: ‘cap’ was declared here
 2358 |  struct avdtp_service_capability *cap;
      |                                   ^~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:8640: profiles/audio/bluetoothd-avdtp.o] Error 1
make: *** [Makefile:4175: all] Error 2




---
Regards,
Linux Bluetooth
Luiz Augusto von Dentz Nov. 15, 2021, 7:42 p.m. UTC | #2 
Hi Kiran,

On Mon, Nov 15, 2021 at 1:36 AM Kiran K <kiran.k@intel.com> wrote:
>
> Define a flag in struct avdtp and set it based on
> the definition of env variable USE_OFFLOAD
> ---
>  profiles/audio/avdtp.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
> index d3dfbf96dda3..b6feac0ba4d5 100644
> --- a/profiles/audio/avdtp.c
> +++ b/profiles/audio/avdtp.c
> @@ -409,6 +409,9 @@ struct avdtp {
>
>         /* Attempt stream setup instead of disconnecting */
>         gboolean stream_setup;
> +
> +       /* use offload for transport */
> +       gboolean use_offload;
>  };
>
>  static GSList *state_callbacks = NULL;
> @@ -2425,6 +2428,7 @@ struct avdtp *avdtp_new(GIOChannel *chan, struct btd_device *device,
>                                                         struct queue *lseps)
>  {
>         struct avdtp *session;
> +       char *use_offload;
>
>         session = g_new0(struct avdtp, 1);
>
> @@ -2436,6 +2440,10 @@ struct avdtp *avdtp_new(GIOChannel *chan, struct btd_device *device,
>
>         session->version = get_version(session);
>
> +       use_offload = getenv("USE_OFFLOAD");
> +       if (use_offload && !strncmp(use_offload, "1", 1))
> +               session->use_offload = TRUE;
> +

We already have a configuration for experimental flags:

https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/src/main.conf#n118

So you just have to check if experimental is enabled, or the offload
UUID, in adapter.c, also perhaps we should have something like
btd_adapter_experimental_is_enabled(adapter, uuid) so it would take
care of doing all the checking if that had been enabled in the kernel
or not.

>         if (!chan)
>                 return session;
>
> --
> 2.17.1
>
Luiz Augusto von Dentz Nov. 15, 2021, 7:44 p.m. UTC | #3 
Hi Kiran,

On Mon, Nov 15, 2021 at 11:42 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Kiran,
>
> On Mon, Nov 15, 2021 at 1:36 AM Kiran K <kiran.k@intel.com> wrote:
> >
> > Define a flag in struct avdtp and set it based on
> > the definition of env variable USE_OFFLOAD
> > ---
> >  profiles/audio/avdtp.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
> > index d3dfbf96dda3..b6feac0ba4d5 100644
> > --- a/profiles/audio/avdtp.c
> > +++ b/profiles/audio/avdtp.c
> > @@ -409,6 +409,9 @@ struct avdtp {
> >
> >         /* Attempt stream setup instead of disconnecting */
> >         gboolean stream_setup;
> > +
> > +       /* use offload for transport */
> > +       gboolean use_offload;
> >  };
> >
> >  static GSList *state_callbacks = NULL;
> > @@ -2425,6 +2428,7 @@ struct avdtp *avdtp_new(GIOChannel *chan, struct btd_device *device,
> >                                                         struct queue *lseps)
> >  {
> >         struct avdtp *session;
> > +       char *use_offload;
> >
> >         session = g_new0(struct avdtp, 1);
> >
> > @@ -2436,6 +2440,10 @@ struct avdtp *avdtp_new(GIOChannel *chan, struct btd_device *device,
> >
> >         session->version = get_version(session);
> >
> > +       use_offload = getenv("USE_OFFLOAD");
> > +       if (use_offload && !strncmp(use_offload, "1", 1))
> > +               session->use_offload = TRUE;
> > +
>
> We already have a configuration for experimental flags:
>
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/src/main.conf#n118

Correction, we may need to introduce yet another experimental UUID
given the UUID above is just about codec offload not MSFT A2DP offload
which may require a completely different set of commands.

> So you just have to check if experimental is enabled, or the offload
> UUID, in adapter.c, also perhaps we should have something like
> btd_adapter_experimental_is_enabled(adapter, uuid) so it would take
> care of doing all the checking if that had been enabled in the kernel
> or not.
>
> >         if (!chan)
> >                 return session;
> >
> > --
> > 2.17.1
> >
>
>
> --
> Luiz Augusto von Dentz
K, Kiran Nov. 19, 2021, 8:12 a.m. UTC | #4 
Hi Luiz,

> -----Original Message-----
> From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
> Sent: Tuesday, November 16, 2021 1:15 AM
> To: K, Kiran <kiran.k@intel.com>
> Cc: linux-bluetooth@vger.kernel.org; Srivatsa, Ravishankar
> <ravishankar.srivatsa@intel.com>; Tumkur Narayan, Chethan
> <chethan.tumkur.narayan@intel.com>; Von Dentz, Luiz
> <luiz.von.dentz@intel.com>
> Subject: Re: [PATCH v1 1/5] avdtp: Add a flag in struct avdtp to control a2dp
> offload
> 
> Hi Kiran,
> 
> On Mon, Nov 15, 2021 at 11:42 AM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > Hi Kiran,
> >
> > On Mon, Nov 15, 2021 at 1:36 AM Kiran K <kiran.k@intel.com> wrote:
> > >
> > > Define a flag in struct avdtp and set it based on the definition of
> > > env variable USE_OFFLOAD
> > > ---
> > >  profiles/audio/avdtp.c | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > >
> > > diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c index
> > > d3dfbf96dda3..b6feac0ba4d5 100644
> > > --- a/profiles/audio/avdtp.c
> > > +++ b/profiles/audio/avdtp.c
> > > @@ -409,6 +409,9 @@ struct avdtp {
> > >
> > >         /* Attempt stream setup instead of disconnecting */
> > >         gboolean stream_setup;
> > > +
> > > +       /* use offload for transport */
> > > +       gboolean use_offload;
> > >  };
> > >
> > >  static GSList *state_callbacks = NULL; @@ -2425,6 +2428,7 @@ struct
> > > avdtp *avdtp_new(GIOChannel *chan, struct btd_device *device,
> > >                                                         struct queue
> > > *lseps)  {
> > >         struct avdtp *session;
> > > +       char *use_offload;
> > >
> > >         session = g_new0(struct avdtp, 1);
> > >
> > > @@ -2436,6 +2440,10 @@ struct avdtp *avdtp_new(GIOChannel *chan,
> > > struct btd_device *device,
> > >
> > >         session->version = get_version(session);
> > >
> > > +       use_offload = getenv("USE_OFFLOAD");
> > > +       if (use_offload && !strncmp(use_offload, "1", 1))
> > > +               session->use_offload = TRUE;
> > > +
> >
> > We already have a configuration for experimental flags:
> >
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/src/main.conf#
> > n118
> 
> Correction, we may need to introduce yet another experimental UUID given
> the UUID above is just about codec offload not MSFT A2DP offload which
> may require a completely different set of commands.
> 
Ok. I will introduce a new UUID for a2dp offload codecs.

> > So you just have to check if experimental is enabled, or the offload
> > UUID, in adapter.c, also perhaps we should have something like
> > btd_adapter_experimental_is_enabled(adapter, uuid) so it would take
> > care of doing all the checking if that had been enabled in the kernel
> > or not.
> >
> > >         if (!chan)
> > >                 return session;
> > >
> > > --
> > > 2.17.1
> > >
> >
> >
> > --
> > Luiz Augusto von Dentz
> 
> 
> 
> --
> Luiz Augusto von Dentz

Thanks,
Kiran
diff mbox series

Patch

diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index d3dfbf96dda3..b6feac0ba4d5 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -409,6 +409,9 @@  struct avdtp {
 
 	/* Attempt stream setup instead of disconnecting */
 	gboolean stream_setup;
+
+	/* use offload for transport */
+	gboolean use_offload;
 };
 
 static GSList *state_callbacks = NULL;
@@ -2425,6 +2428,7 @@  struct avdtp *avdtp_new(GIOChannel *chan, struct btd_device *device,
 							struct queue *lseps)
 {
 	struct avdtp *session;
+	char *use_offload;
 
 	session = g_new0(struct avdtp, 1);
 
@@ -2436,6 +2440,10 @@  struct avdtp *avdtp_new(GIOChannel *chan, struct btd_device *device,
 
 	session->version = get_version(session);
 
+	use_offload = getenv("USE_OFFLOAD");
+	if (use_offload && !strncmp(use_offload, "1", 1))
+		session->use_offload = TRUE;
+
 	if (!chan)
 		return session;