[BlueZ] adapter: Fix the pending changing flags check

Commit Message

Ludovico de Nittis Jan. 29, 2025, 5:23 p.m. UTC

When checking if the new desired device flags are already pending, we
should compare them against the XOR of current flags and desired flags,
i.e. the flags that are going to change.

For example, let's assume the following situation:
- We have a device with `current_flags == DEVICE_FLAG_REMOTE_WAKEUP`
(i.e. 1)
- We want to disable the `wake_allowed` property
- `device_set_wake_allowed()` will call `adapter_set_device_flags()`
with `flags == 0`
- When in `adapter_set_device_flags()`, we'll have:
  - current == 1
  - pending == 0
  - flags == 0
In this situation `flags == (flags & pending)` would not return what
we'd expect.
---
 src/adapter.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

[e-mail] Jan. 29, 2025, 6:30 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.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=929128

---Test result---

Test Summary:
CheckPatch                    PENDING   0.34 seconds
GitLint                       PENDING   0.22 seconds
BuildEll                      PASS      20.16 seconds
BluezMake                     PASS      1491.56 seconds
MakeCheck                     PASS      13.19 seconds
MakeDistcheck                 PASS      156.72 seconds
CheckValgrind                 PASS      212.20 seconds
CheckSmatch                   PASS      269.67 seconds
bluezmakeextell               PASS      97.29 seconds
IncrementalBuild              PENDING   0.24 seconds
ScanBuild                     PASS      847.84 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth

[e-mail] Jan. 30, 2025, 5:30 p.m. UTC | #2

Hello:

This patch was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <[e-mail]>:

On Wed, 29 Jan 2025 18:23:16 +0100 you wrote:
> When checking if the new desired device flags are already pending, we
> should compare them against the XOR of current flags and desired flags,
> i.e. the flags that are going to change.
> 
> For example, let's assume the following situation:
> - We have a device with `current_flags == DEVICE_FLAG_REMOTE_WAKEUP`
> (i.e. 1)
> - We want to disable the `wake_allowed` property
> - `device_set_wake_allowed()` will call `adapter_set_device_flags()`
> with `flags == 0`
> - When in `adapter_set_device_flags()`, we'll have:
>   - current == 1
>   - pending == 0
>   - flags == 0
> In this situation `flags == (flags & pending)` would not return what
> we'd expect.
> 
> [...]

Here is the summary with links:
  - [BlueZ] adapter: Fix the pending changing flags check
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=0acdf186fcde

You are awesome, thank you!

Patch

diff --git a/src/adapter.c b/src/adapter.c
index 5d4117a49..e55fb7f3b 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -5684,6 +5684,7 @@  void adapter_set_device_flags(struct btd_adapter *adapter,
 				mgmt_request_func_t func, void *user_data)
 {
 	struct mgmt_cp_set_device_flags cp;
+	uint32_t current = btd_device_get_current_flags(device);
 	uint32_t supported = btd_device_get_supported_flags(device);
 	uint32_t pending = btd_device_get_pending_flags(device);
 	const bdaddr_t *bdaddr;
@@ -5694,7 +5695,7 @@  void adapter_set_device_flags(struct btd_adapter *adapter,
 		return;
 
 	/* Check if changing flags are pending */
-	if (flags == (flags & pending))
+	if ((current ^ flags) == (flags & pending))
 		return;
 
 	/* Set Device Privacy Mode if it has not set the flag yet. */