[BlueZ] avrcp: Fix unregister AVRCP player

Commit Message

Huang-Huang Bao May 20, 2021, 3:13 a.m. UTC

'notify_addressed_player_changed()' expected to be called with
'player->changed_id' set to what 'g_idle_add()' returns.

    player->changed_id = g_idle_add(notify_addressed_player_changed, player);

And 'avrcp_player_event()' relies on 'player->changed_id' to perform
Addressed Player Changed notification. However,
'avrcp_unregister_player()' calls 'notify_addressed_player_changed()'
without adding it to the main loop and set 'player->changed_id'. To
make 'notify_addressed_player_changed()' can be called without set
'player->changed_id' flag. We add antoher flag
'player->addressed_changing' to indicate addressed player changing.

Fixes https://github.com/bluez/bluez/issues/142
---
 profiles/audio/avrcp.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

[e-mail] May 20, 2021, 3:54 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=485435

---Test result---

Test Summary:
CheckPatch                    FAIL      0.32 seconds
GitLint                       PASS      0.11 seconds
Prep - Setup ELL              PASS      40.94 seconds
Build - Prep                  PASS      0.11 seconds
Build - Configure             PASS      7.27 seconds
Build - Make                  PASS      176.69 seconds
Make Check                    PASS      9.45 seconds
Make Distcheck                PASS      210.25 seconds
Build w/ext ELL - Configure   PASS      7.30 seconds
Build w/ext ELL - Make        PASS      166.44 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script with rule in .checkpatch.conf
Output:
avrcp: Fix unregister AVRCP player
WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#9: 
    player->changed_id = g_idle_add(notify_addressed_player_changed, player);

WARNING:LONG_LINE: line length of 87 exceeds 80 columns
#38: FILE: profiles/audio/avrcp.c:796:
+	if (id != AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED && player->addressed_changing) {

- total: 0 errors, 2 warnings, 30 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.

"[PATCH] avrcp: Fix unregister AVRCP player" 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: GitLint - PASS
Desc: Run gitlint with rule in .gitlint

##############################
Test: Prep - Setup ELL - PASS
Desc: Clone, build, and install ELL

##############################
Test: Build - Prep - PASS
Desc: Prepare environment for build

##############################
Test: Build - Configure - PASS
Desc: Configure the BlueZ source tree

##############################
Test: Build - Make - PASS
Desc: Build the BlueZ source tree

##############################
Test: Make Check - PASS
Desc: Run 'make check'

##############################
Test: Make Distcheck - PASS
Desc: Run distcheck to check the distribution

##############################
Test: Build w/ext ELL - Configure - PASS
Desc: Configure BlueZ source with '--enable-external-ell' configuration

##############################
Test: Build w/ext ELL - Make - PASS
Desc: Build BlueZ source with '--enable-external-ell' configuration



---
Regards,
Linux Bluetooth

Patch

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 58d30b24d..039ac8924 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -239,6 +239,7 @@  struct avrcp_player {
 	uint8_t *features;
 	char *path;
 	guint changed_id;
+	bool addressed_changing;
 
 	struct pending_list_items *p;
 	char *change_path;
@@ -792,7 +793,7 @@  void avrcp_player_event(struct avrcp_player *player, uint8_t id,
 
 	DBG("id=%u", id);
 
-	if (id != AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED && player->changed_id) {
+	if (id != AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED && player->addressed_changing) {
 		code = AVC_CTYPE_REJECTED;
 		size = 1;
 		pdu->params[0] = AVRCP_STATUS_ADDRESSED_PLAYER_CHANGED;
@@ -1794,6 +1795,8 @@  static gboolean notify_addressed_player_changed(gpointer user_data)
 				};
 	uint8_t i;
 
+	player->addressed_changing = true;
+
 	avrcp_player_event(player, AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED, NULL);
 
 	/*
@@ -1804,6 +1807,7 @@  static gboolean notify_addressed_player_changed(gpointer user_data)
 	for (i = 0; i < sizeof(events); i++)
 		avrcp_player_event(player, events[i], NULL);
 
+	player->addressed_changing = false;
 	player->changed_id = 0;
 
 	return FALSE;