| Message ID | 20220829203122.51343-2-frederic.danis@collabora.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | profiles: Add remote endpoint path to SelectProperties | expand |
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
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 >
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); + 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, 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,