spp: fix resource leaks and add SDP discover timeout

[chejinxian]

Fix SPP UUID-based connection resource management issues:

1. Fix conn_id leak when malloc fails in alloc_new_device (bug: v/88807)
2. Change UNKNOWN_SERVER_CHANNEL_NUM to 0 per RFCOMM spec (bug: v/88808)
3. Fix resource leak when RFCOMM connect fails after SDP discover (bug: v/88809)
4. Add SDP discover timeout and fix resource cleanup (bug: v/88810)

When SPP connects via 128-bit UUID, the SDP discovery phase had multiple
resource management defects that could cause connection resource leaks,
subsequent connections failing with BT_STATUS_BUSY, or connections hanging
indefinitely when the remote device does not respond to SDP.

depends-on: [open-vela/external_zblue/pull/211]

[Copilot]

Pull request overview

Fixes multiple resource-management issues in SPP UUID-based connection flow (Zephyr SAL), including preventing connection ID leaks, aligning the “unknown SCN” sentinel with RFCOMM expectations, and adding an SDP discovery timeout to avoid indefinite hangs when the peer doesn’t respond.

Changes:

• Add SDP discovery timeout + timer cleanup hooks in Zephyr SPP SAL, and adjust cleanup on SDP/RFCOMM failure paths.
• Fix conn_id leak on allocation failure in SPP profile service.
• Update UNKNOWN_SERVER_CHANNEL_NUM to 0 and adjust related header docs; minor GATT dump refactor.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
service/stacks/zephyr/sal_spp_interface.c	Adds SDP discovery timeout timer and new cleanup paths for SDP/RFCOMM failures.
service/profiles/spp/spp_service.c	Frees allocated conn_id when malloc(sizeof(spp_device_t)) fails.
service/profiles/gatt/gattc_service.c	Minor refactor to inline a temporary variable in dump loop.
framework/include/bt_spp.h	Updates unknown SCN sentinel to 0 and improves its documentation.

Comments suppressed due to low confidence (1)

service/stacks/zephyr/sal_spp_interface.c:792

• sdp_discovered_cb() dereferences result (result->resp_buf) without checking whether result is NULL. Other SDP discovery callbacks in this repo (e.g., HFP) treat result == NULL as a valid “no matching service / discovery complete” signal. Add a !result check before accessing fields and handle the failure case by cleaning up and disconnecting/removing the pending SPP connection.

static uint8_t sdp_discovered_cb(struct bt_conn* conn, struct bt_sdp_client_result* result,
    const struct bt_sdp_discover_params* param)
{
    int err;
    uint8_t ret = BT_SDP_DISCOVER_UUID_STOP;
    uint16_t scn;
    sal_spp_connection_t* spp_conn;

    spp_conn = spp_find_connection_by_sdp_param(conn, param);
    if (!spp_conn) {
        BT_LOGE("SPP connection not found for conn");
        return BT_SDP_DISCOVER_UUID_STOP;
    }

    spp_sdp_cancel_timer(spp_conn->spp_client);

    if (!result->resp_buf) {
        BT_LOGE("SPP SDP discover response buffer is null");
        ret = BT_SDP_DISCOVER_UUID_CONTINUE;
        goto fail;

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

spp_sdp_timeout_handler() sets sdp_timer to NULL but never cancels/closes the libuv timer handle. Since service_loop_cancel_timer() is what actually uv_close()s the timer and frees it (via handle_close_cb), this will leak the timer after it fires, and spp_connection_free() won't be able to cancel it later because the pointer was cleared. Cancel/close the timer in the timeout handler (e.g., call service_loop_cancel_timer(timer) or spp_sdp_cancel_timer() before NULLing) and avoid clearing the pointer until after cancel.

Suggested change

	spp_conn->spp_client->sdp_timer = NULL;
	if (spp_conn->spp_client->sdp_timer) {
	service_loop_cancel_timer(spp_conn->spp_client->sdp_timer);
	spp_conn->spp_client->sdp_timer = NULL;
	}

[Copilot]

The SDP timeout is canceled immediately on entry to sdp_discovered_cb(). If the callback returns BT_SDP_DISCOVER_UUID_CONTINUE (e.g., when resp_buf is NULL or bt_sdp_get_proto_param() fails), discovery can continue with no timeout running, which undermines the added hang protection. Consider canceling the timer only once discovery is definitively finishing (STOP/success) or restarting/rescheduling the timer when continuing.

[Copilot]

The new UNKNOWN_SERVER_CHANNEL_NUM doc note says “SCN valid range is 1-30”, but other SPP API docs in this header still describe SCN as “range in 1-28”. Please reconcile these statements (e.g., mention that RFCOMM allows 1-30 but this API/service reserves 29–30, so callers should use 1–28).

Suggested change

	* @note SCN valid range is 1-30 per RFCOMM spec, 0 means unspecified
	* @note RFCOMM defines SCN values in the range 1-30; this API reserves 29-30 for
	* internal use, so callers must use SCN values in the range 1-28. 0 means
	* unspecified.

[zhongzhijie1]

LGTM. conn_id leak fix、UNKNOWN_SERVER_CHANNEL_NUM 改 0 符合 RFCOMM spec、SDP discover timeout + cancel + 资源清理完整。

The merge-base changed after approval.