* RE: profiles: Add remote endpoint path to SelectProperties
2022-08-29 20:31 ` [PATCH BlueZ 1/2] " Frédéric Danis
@ 2022-08-29 21:29 ` bluez.test.bot
2022-08-29 21:55 ` [PATCH BlueZ 1/2] " Luiz Augusto von Dentz
1 sibling, 0 replies; 5+ messages in thread
From: bluez.test.bot @ 2022-08-29 21:29 UTC (permalink / raw)
To: linux-bluetooth, frederic.danis
[-- Attachment #1: Type: text/plain, Size: 3091 bytes --]
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=672169
---Test result---
Test Summary:
CheckPatch FAIL 2.95 seconds
GitLint PASS 2.01 seconds
Prep - Setup ELL PASS 27.91 seconds
Build - Prep PASS 0.84 seconds
Build - Configure PASS 8.96 seconds
Build - Make PASS 1039.25 seconds
Make Check PASS 11.03 seconds
Make Check w/Valgrind PASS 276.22 seconds
Make Distcheck PASS 238.53 seconds
Build w/ext ELL - Configure PASS 8.61 seconds
Build w/ext ELL - Make PASS 82.44 seconds
Incremental Build w/ patches PASS 201.30 seconds
Scan Build WARNING 580.56 seconds
Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script with rule in .checkpatch.conf
Output:
[BlueZ,1/2] profiles: Add remote endpoint path to SelectProperties
WARNING:LONG_LINE: line length of 86 exceeds 80 columns
#99: FILE: profiles/audio/media.c:921:
+ dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH, &remote_ep_path);
/github/workspace/src/12958401.patch total: 0 errors, 1 warnings, 51 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/12958401.patch has style problems, please review.
NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
##############################
Test: Scan Build - WARNING
Desc: Run Scan Build with patches
Output:
*****************************************************************************
The bugs reported by the scan-build may or may not be caused by your patches.
Please check the list and fix the bugs if they are caused by your patch.
*****************************************************************************
profiles/audio/media.c:1453:6: warning: 8th function call argument is an uninitialized value
if (media_endpoint_create(adapter, sender, path, uuid, delay_reporting,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
profiles/audio/media.c:2999:3: warning: Use of memory after it is freed
release_endpoint(adapter->endpoints->data);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
profiles/audio/media.c:3002:3: warning: Use of memory after it is freed
media_player_destroy(adapter->players->data);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
3 warnings generated.
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH BlueZ 1/2] profiles: Add remote endpoint path to SelectProperties
2022-08-29 20:31 ` [PATCH BlueZ 1/2] " Frédéric Danis
2022-08-29 21:29 ` bluez.test.bot
@ 2022-08-29 21:55 ` Luiz Augusto von Dentz
1 sibling, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2022-08-29 21:55 UTC (permalink / raw)
To: Frédéric Danis; +Cc: linux-bluetooth@vger.kernel.org
Hi Frédéric,
On Mon, Aug 29, 2022 at 1:36 PM Frédéric Danis
<frederic.danis@collabora.com> wrote:
>
> ---
> profiles/audio/bap.c | 2 +-
> profiles/audio/media.c | 3 +++
> src/shared/bap.c | 2 ++
> src/shared/bap.h | 2 ++
> 4 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c
> index d388afe56..cf27ec0ae 100644
> --- a/profiles/audio/bap.c
> +++ b/profiles/audio/bap.c
> @@ -613,7 +613,7 @@ static bool pac_found(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
>
> /* TODO: Cache LRU? */
> if (btd_service_is_initiator(service))
> - bt_bap_select(lpac, rpac, select_cb, ep);
> + bt_bap_select(lpac, rpac, ep->path, select_cb, ep);
>
> return true;
> }
> diff --git a/profiles/audio/media.c b/profiles/audio/media.c
> index ff3fa197b..8d777eedd 100644
> --- a/profiles/audio/media.c
> +++ b/profiles/audio/media.c
> @@ -891,6 +891,7 @@ done:
>
> static int pac_select(struct bt_bap_pac *pac, struct bt_bap_pac_qos *qos,
> struct iovec *caps, struct iovec *metadata,
> + const char *remote_ep_path,
> bt_bap_pac_select_t cb, void *cb_data, void *user_data)
> {
> struct media_endpoint *endpoint = user_data;
> @@ -917,6 +918,8 @@ static int pac_select(struct bt_bap_pac *pac, struct bt_bap_pac_qos *qos,
>
> dbus_message_iter_init_append(msg, &iter);
>
> + dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH, &remote_ep_path);
> +
If the endpoint object is not part of the dictionary then you will
have to fix client/player.c as well since the signature will be
changing, so I think it is better we include it as part of properties
itself.
> dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY, "{sv}", &dict);
>
> g_dbus_dict_append_basic_array(&dict, DBUS_TYPE_STRING, &key,
> diff --git a/src/shared/bap.c b/src/shared/bap.c
> index 8edc7b72e..691fec2fa 100644
> --- a/src/shared/bap.c
> +++ b/src/shared/bap.c
> @@ -4058,6 +4058,7 @@ static bool match_pac(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
> }
>
> int bt_bap_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
> + const char *remote_ep_path,
> bt_bap_pac_select_t func, void *user_data)
> {
> if (!lpac || !rpac || !func)
> @@ -4067,6 +4068,7 @@ int bt_bap_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
> return -EOPNOTSUPP;
>
> lpac->ops->select(lpac, &rpac->qos, rpac->data, rpac->metadata,
> + remote_ep_path,
> func, user_data, lpac->user_data);
>
> return 0;
> diff --git a/src/shared/bap.h b/src/shared/bap.h
> index ff4bac330..da5fe1431 100644
> --- a/src/shared/bap.h
> +++ b/src/shared/bap.h
> @@ -122,6 +122,7 @@ struct bt_bap_pac *bt_bap_add_pac(struct gatt_db *db, const char *name,
> struct bt_bap_pac_ops {
> int (*select) (struct bt_bap_pac *pac, struct bt_bap_pac_qos *qos,
> struct iovec *caps, struct iovec *metadata,
> + const char *remote_ep_path,
Hmm, Id avoid passing D-Bus specific path here since this API is
suppose to be generic and at some point we might want to create a unit
tests that don't involve the daemon, so instead I think it is better
that we pass the rpac and addition to the lpac and then perhaps have a
function which can attach custom user_data to the rpac e.g.: void
bt_bap_pac_set_user_data(void *); void *bt_bap_pac_get_user_data();
> bt_bap_pac_select_t cb, void *cb_data, void *user_data);
> int (*config) (struct bt_bap_stream *stream, struct iovec *cfg,
> struct bt_bap_qos *qos, bt_bap_pac_config_t cb,
> @@ -188,6 +189,7 @@ int bt_bap_pac_get_codec(struct bt_bap_pac *pac, uint8_t *id,
>
> /* Stream related functions */
> int bt_bap_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
> + const char *remote_ep_path,
> bt_bap_pac_select_t func, void *user_data);
>
> struct bt_bap_stream *bt_bap_config(struct bt_bap *bap,
> --
> 2.25.1
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 5+ messages in thread