diff mbox series

[v3,1/4] a2dp: Fix caching endpoints for unknown version

Message ID 20200519200345.217345-1-luiz.dentz@gmail.com (mailing list archive)
State Accepted
Delegated to: Luiz Von Dentz
Headers show
Series [v3,1/4] a2dp: Fix caching endpoints for unknown version | expand

Commit Message

Luiz Augusto von Dentz May 19, 2020, 8:03 p.m. UTC
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Don't cache the capabilities of endpoints which the version is unknown
since so capabilities may not be available in such case.
---
 profiles/audio/a2dp.c  | 11 +++++++++--
 profiles/audio/avdtp.c |  7 ++++++-
 profiles/audio/avdtp.h |  1 +
 3 files changed, 16 insertions(+), 3 deletions(-)

Comments

bluez.test.bot@gmail.com May 19, 2020, 8:39 p.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.
While we are preparing for reviewing the patches, we found the following
issue/warning.

Test Result:
makecheck Failed

Outputs:
./test-driver: line 107: 14841 Aborted                 (core dumped) "$@" > $log_file 2>&1
make[3]: *** [Makefile:9726: test-suite.log] Error 1
make[2]: *** [Makefile:9834: check-TESTS] Error 2
make[1]: *** [Makefile:10228: check-am] Error 2
make: *** [Makefile:10230: check] Error 2



---
Regards,
Linux Bluetooth
Luiz Augusto von Dentz May 19, 2020, 9:06 p.m. UTC | #2
Hi Tedd,

On Tue, May 19, 2020 at 1:39 PM <bluez.test.bot@gmail.com> wrote:
>
>
> 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.
> While we are preparing for reviewing the patches, we found the following
> issue/warning.
>
> Test Result:
> makecheck Failed
>
> Outputs:
> ./test-driver: line 107: 14841 Aborted                 (core dumped) "$@" > $log_file 2>&1
> make[3]: *** [Makefile:9726: test-suite.log] Error 1
> make[2]: *** [Makefile:9834: check-TESTS] Error 2
> make[1]: *** [Makefile:10228: check-am] Error 2
> make: *** [Makefile:10230: check] Error 2

Can you give a look why CI is core dumping here, also perhaps we
should inspect the test-suite.log when this happens.
An, Tedd May 19, 2020, 9:43 p.m. UTC | #3
Hi Luiz

The failure is from "test-midi" from the output. However, the current script didn't save the test-suite.log.
I have another test setup on a different account but the test passed on that setup with same patch set. So it could be the false positive as Brian mentioned.

Let me make a change to
- capture/save the "test-suite.log" if there is a failure

Also, I ran again the it passed at the 2nd time. 
I will disable the test-midi until we fix this issue.

Regards,
Tedd

On 5/19/20, 2:07 PM, "linux-bluetooth-owner@vger.kernel.org on behalf of Luiz Augusto von Dentz" <linux-bluetooth-owner@vger.kernel.org on behalf of luiz.dentz@gmail.com> wrote:

    Hi Tedd,

    On Tue, May 19, 2020 at 1:39 PM <bluez.test.bot@gmail.com> wrote:
    >
    >
    > 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.
    > While we are preparing for reviewing the patches, we found the following
    > issue/warning.
    >
    > Test Result:
    > makecheck Failed
    >
    > Outputs:
    > ./test-driver: line 107: 14841 Aborted                 (core dumped) "$@" > $log_file 2>&1
    > make[3]: *** [Makefile:9726: test-suite.log] Error 1
    > make[2]: *** [Makefile:9834: check-TESTS] Error 2
    > make[1]: *** [Makefile:10228: check-am] Error 2
    > make: *** [Makefile:10230: check] Error 2

    Can you give a look why CI is core dumping here, also perhaps we
    should inspect the test-suite.log when this happens.

    -- 
    Luiz Augusto von Dentz
Luiz Augusto von Dentz May 21, 2020, 4:22 p.m. UTC | #4
Hi,

On Tue, May 19, 2020 at 1:03 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> Don't cache the capabilities of endpoints which the version is unknown
> since so capabilities may not be available in such case.
> ---
>  profiles/audio/a2dp.c  | 11 +++++++++--
>  profiles/audio/avdtp.c |  7 ++++++-
>  profiles/audio/avdtp.h |  1 +
>  3 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> index a2ce3204d..15e211b95 100644
> --- a/profiles/audio/a2dp.c
> +++ b/profiles/audio/a2dp.c
> @@ -2667,15 +2667,22 @@ static void discover_cb(struct avdtp *session, GSList *seps,
>                                 struct avdtp_error *err, void *user_data)
>  {
>         struct a2dp_setup *setup = user_data;
> +       uint16_t version = avdtp_get_version(session);
>
> -       DBG("err %p", err);
> +       DBG("version 0x%04x err %p", version, err);
>
>         setup->seps = seps;
>         setup->err = err;
>
>         if (!err) {
>                 g_slist_foreach(seps, register_remote_sep, setup->chan);
> -               store_remote_seps(setup->chan);
> +
> +               /* Only store version has been initialized as features like
> +                * Delay Reporting may not be queried if the version in
> +                * unknown.
> +                */
> +               if (version)
> +                       store_remote_seps(setup->chan);
>         }
>
>         finalize_discover(setup);
> diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
> index b632e41c5..1fd2be051 100644
> --- a/profiles/audio/avdtp.c
> +++ b/profiles/audio/avdtp.c
> @@ -2256,7 +2256,7 @@ static uint16_t get_version(struct avdtp *session)
>         const sdp_record_t *rec;
>         sdp_list_t *protos;
>         sdp_data_t *proto_desc;
> -       uint16_t ver = 0x0100;
> +       uint16_t ver = 0x0000;
>
>         rec = btd_device_get_record(session->device, A2DP_SINK_UUID);
>         if (!rec)
> @@ -2396,6 +2396,11 @@ struct avdtp *avdtp_new(GIOChannel *chan, struct btd_device *device,
>         return session;
>  }
>
> +uint16_t avdtp_get_version(struct avdtp *session)
> +{
> +       return session->version;
> +}
> +
>  static GIOChannel *l2cap_connect(struct avdtp *session)
>  {
>         GError *err = NULL;
> diff --git a/profiles/audio/avdtp.h b/profiles/audio/avdtp.h
> index ad2cb9bcb..f1e51d4e3 100644
> --- a/profiles/audio/avdtp.h
> +++ b/profiles/audio/avdtp.h
> @@ -310,3 +310,4 @@ struct avdtp_server *avdtp_get_server(struct avdtp_local_sep *lsep);
>
>  struct avdtp *avdtp_new(GIOChannel *chan, struct btd_device *device,
>                                                         struct queue *lseps);
> +uint16_t avdtp_get_version(struct avdtp *session);
> --
> 2.25.3

Pushed.
diff mbox series

Patch

diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index a2ce3204d..15e211b95 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -2667,15 +2667,22 @@  static void discover_cb(struct avdtp *session, GSList *seps,
 				struct avdtp_error *err, void *user_data)
 {
 	struct a2dp_setup *setup = user_data;
+	uint16_t version = avdtp_get_version(session);
 
-	DBG("err %p", err);
+	DBG("version 0x%04x err %p", version, err);
 
 	setup->seps = seps;
 	setup->err = err;
 
 	if (!err) {
 		g_slist_foreach(seps, register_remote_sep, setup->chan);
-		store_remote_seps(setup->chan);
+
+		/* Only store version has been initialized as features like
+		 * Delay Reporting may not be queried if the version in
+		 * unknown.
+		 */
+		if (version)
+			store_remote_seps(setup->chan);
 	}
 
 	finalize_discover(setup);
diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index b632e41c5..1fd2be051 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -2256,7 +2256,7 @@  static uint16_t get_version(struct avdtp *session)
 	const sdp_record_t *rec;
 	sdp_list_t *protos;
 	sdp_data_t *proto_desc;
-	uint16_t ver = 0x0100;
+	uint16_t ver = 0x0000;
 
 	rec = btd_device_get_record(session->device, A2DP_SINK_UUID);
 	if (!rec)
@@ -2396,6 +2396,11 @@  struct avdtp *avdtp_new(GIOChannel *chan, struct btd_device *device,
 	return session;
 }
 
+uint16_t avdtp_get_version(struct avdtp *session)
+{
+	return session->version;
+}
+
 static GIOChannel *l2cap_connect(struct avdtp *session)
 {
 	GError *err = NULL;
diff --git a/profiles/audio/avdtp.h b/profiles/audio/avdtp.h
index ad2cb9bcb..f1e51d4e3 100644
--- a/profiles/audio/avdtp.h
+++ b/profiles/audio/avdtp.h
@@ -310,3 +310,4 @@  struct avdtp_server *avdtp_get_server(struct avdtp_local_sep *lsep);
 
 struct avdtp *avdtp_new(GIOChannel *chan, struct btd_device *device,
 							struct queue *lseps);
+uint16_t avdtp_get_version(struct avdtp *session);