Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bluetooth: L2CAP_BR: Support dynamic PSM allocation #83783

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lylezhu2012
Copy link
Contributor

In the function bt_l2cap_br_server_register(), the PSM cannot be dynamic allocated. And only pre-set PSM is supported.

Improve the function bt_l2cap_br_server_register() to support the dynamic PSM allocation if the passed PSM is zero.

LOG_DBG("PSM already registered");
return -EADDRINUSE;
if (!server->psm) {
err = bt_l2cap_br_allocate_psm();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be a little bit cleaner if the PSM was returned through a pointer parameter:

err = bt_l2cap_br_allocate_psm(&server->psm);
if (err) {
	return err;
}

Then you don't need the explicit typecast from int to uint16_t. Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well. But I think the positive value of err is used in current scenario, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, then you don't "take advantage" of the positive values anymore, which is the way most APIs that return int errors behave. I'm just saying that from an API design, usability and intuitiveness perspective I feel like it's clearer to split out the returned object or value into a separate pointer parameter and keep the return value purely for error reporting. If you compare my above snippet to the current code it's also less lines of code in addition to eliminating the explicit typecast. C doesn't have any safeguards when doing explicit typecasts, i.e. it happily accepts whatever you ask it, so it's often better if you can write code that doesn't need any such typecasts to begin with.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Updated.

In the function `bt_l2cap_br_server_register()`, the PSM cannot be
dynamic allocated. And only pre-set PSM is supported.

Improve the function `bt_l2cap_br_server_register()` to support the
dynamic PSM allocation if the passed PSM is zero.

Signed-off-by: Lyle Zhu <[email protected]>
@lylezhu2012 lylezhu2012 force-pushed the l2cap_support_dynamic_psm_allocation branch from 6a797c3 to bed7cd0 Compare January 10, 2025 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants