[BlueZ,v2,1/2] btio: Add option for binding iso broadcast address

Commit Message

Iulia Tanasescu June 28, 2023, 3:07 p.m. UTC

This adds the BT_IO_OPT_ISO_BC_ADDR btio option, to allow the user
to set the iso_bc field of an address at bind.
---
 btio/btio.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++-------
 btio/btio.h |  5 +++-
 2 files changed, 70 insertions(+), 11 deletions(-)

Comments

[e-mail] June 28, 2023, 3:50 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=760971

---Test result---

Test Summary:
CheckPatch                    PASS      1.48 seconds
GitLint                       PASS      0.51 seconds
BuildEll                      PASS      34.60 seconds
BluezMake                     FAIL      25.80 seconds
MakeCheck                     FAIL      40.85 seconds
MakeDistcheck                 FAIL      44.29 seconds
CheckValgrind                 FAIL      21.60 seconds
CheckSmatch                   FAIL      29.55 seconds
bluezmakeextell               FAIL      19.27 seconds
IncrementalBuild              FAIL      1069.65 seconds
ScanBuild                     FAIL      746.90 seconds

Details
##############################
Test: BluezMake - FAIL
Desc: Build BlueZ
Output:

src/shared/bass.c: In function ‘bass_handle_add_src_op’:
src/shared/bass.c:835:21: error: ‘bis_sync_bit’ undeclared (first use in this function)
  835 |      bis[num_bis] = bis_sync_bit + 1;
      |                     ^~~~~~~~~~~~
src/shared/bass.c:835:21: note: each undeclared identifier is reported only once for each function it appears in
src/shared/bass.c:855:10: error: ‘bc_addr’ undeclared (first use in this function); did you mean ‘in_addr’?
  855 |   bacpy(&bc_addr.bc_bdaddr, &bcast_src->addr);
      |          ^~~~~~~
      |          in_addr
make[1]: *** [Makefile:8399: src/shared/libshared_mainloop_la-bass.lo] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:4528: all] Error 2
##############################
Test: MakeCheck - FAIL
Desc: Run Bluez Make Check
Output:

src/shared/bass.c: In function ‘bass_handle_add_src_op’:
src/shared/bass.c:835:21: error: ‘bis_sync_bit’ undeclared (first use in this function)
  835 |      bis[num_bis] = bis_sync_bit + 1;
      |                     ^~~~~~~~~~~~
src/shared/bass.c:835:21: note: each undeclared identifier is reported only once for each function it appears in
src/shared/bass.c:855:10: error: ‘bc_addr’ undeclared (first use in this function); did you mean ‘in_addr’?
  855 |   bacpy(&bc_addr.bc_bdaddr, &bcast_src->addr);
      |          ^~~~~~~
      |          in_addr
make[1]: *** [Makefile:8182: src/shared/libshared_glib_la-bass.lo] Error 1
make: *** [Makefile:11863: check] Error 2
##############################
Test: MakeDistcheck - FAIL
Desc: Run Bluez Make Distcheck
Output:

../../src/shared/bass.c: In function ‘bass_handle_add_src_op’:
../../src/shared/bass.c:835:21: error: ‘bis_sync_bit’ undeclared (first use in this function)
  835 |      bis[num_bis] = bis_sync_bit + 1;
      |                     ^~~~~~~~~~~~
../../src/shared/bass.c:835:21: note: each undeclared identifier is reported only once for each function it appears in
../../src/shared/bass.c:855:10: error: ‘bc_addr’ undeclared (first use in this function); did you mean ‘in_addr’?
  855 |   bacpy(&bc_addr.bc_bdaddr, &bcast_src->addr);
      |          ^~~~~~~
      |          in_addr
make[2]: *** [Makefile:8399: src/shared/libshared_mainloop_la-bass.lo] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [Makefile:4528: all] Error 2
make: *** [Makefile:11784: distcheck] Error 1
##############################
Test: CheckValgrind - FAIL
Desc: Run Bluez Make Check with Valgrind
Output:

src/shared/bass.c: In function ‘bass_handle_add_src_op’:
src/shared/bass.c:835:21: error: ‘bis_sync_bit’ undeclared (first use in this function)
  835 |      bis[num_bis] = bis_sync_bit + 1;
      |                     ^~~~~~~~~~~~
src/shared/bass.c:835:21: note: each undeclared identifier is reported only once for each function it appears in
src/shared/bass.c:855:10: error: ‘bc_addr’ undeclared (first use in this function); did you mean ‘in_addr’?
  855 |   bacpy(&bc_addr.bc_bdaddr, &bcast_src->addr);
      |          ^~~~~~~
      |          in_addr
make[1]: *** [Makefile:8399: src/shared/libshared_mainloop_la-bass.lo] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:11863: check] Error 2
##############################
Test: CheckSmatch - FAIL
Desc: Run smatch tool with source
Output:

src/shared/crypto.c:271:21: warning: Variable length array is used.
src/shared/crypto.c:272:23: warning: Variable length array is used.
src/shared/gatt-helpers.c:768:31: warning: Variable length array is used.
src/shared/gatt-helpers.c:830:31: warning: Variable length array is used.
src/shared/gatt-helpers.c:1323:31: warning: Variable length array is used.
src/shared/gatt-helpers.c:1354:23: warning: Variable length array is used.
src/shared/gatt-server.c:276:25: warning: Variable length array is used.
src/shared/gatt-server.c:619:25: warning: Variable length array is used.
src/shared/gatt-server.c:718:25: warning: Variable length array is used.
src/shared/shell.c: note: in included file (through /usr/include/readline/readline.h):
/usr/include/readline/rltypedefs.h:35:23: warning: non-ANSI function declaration of function 'Function'
/usr/include/readline/rltypedefs.h:36:25: warning: non-ANSI function declaration of function 'VFunction'
/usr/include/readline/rltypedefs.h:37:27: warning: non-ANSI function declaration of function 'CPFunction'
/usr/include/readline/rltypedefs.h:38:29: warning: non-ANSI function declaration of function 'CPPFunction'
src/shared/bass.c: In function ‘bass_handle_add_src_op’:
src/shared/bass.c:835:21: error: ‘bis_sync_bit’ undeclared (first use in this function)
  835 |      bis[num_bis] = bis_sync_bit + 1;
      |                     ^~~~~~~~~~~~
src/shared/bass.c:835:21: note: each undeclared identifier is reported only once for each function it appears in
src/shared/bass.c:855:10: error: ‘bc_addr’ undeclared (first use in this function); did you mean ‘in_addr’?
  855 |   bacpy(&bc_addr.bc_bdaddr, &bcast_src->addr);
      |          ^~~~~~~
      |          in_addr
make[1]: *** [Makefile:8399: src/shared/libshared_mainloop_la-bass.lo] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:4528: all] Error 2
##############################
Test: bluezmakeextell - FAIL
Desc: Build Bluez with External ELL
Output:

src/shared/bass.c: In function ‘bass_handle_add_src_op’:
src/shared/bass.c:835:21: error: ‘bis_sync_bit’ undeclared (first use in this function)
  835 |      bis[num_bis] = bis_sync_bit + 1;
      |                     ^~~~~~~~~~~~
src/shared/bass.c:835:21: note: each undeclared identifier is reported only once for each function it appears in
src/shared/bass.c:855:10: error: ‘bc_addr’ undeclared (first use in this function); did you mean ‘in_addr’?
  855 |   bacpy(&bc_addr.bc_bdaddr, &bcast_src->addr);
      |          ^~~~~~~
      |          in_addr
make[1]: *** [Makefile:8399: src/shared/libshared_mainloop_la-bass.lo] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:4528: all] Error 2
##############################
Test: IncrementalBuild - FAIL
Desc: Incremental build with the patches in the series
Output:
[BlueZ,v2,2/2] shared/bass: Introduce Add Source opcode handler

src/shared/bass.c: In function ‘bass_handle_add_src_op’:
src/shared/bass.c:835:21: error: ‘bis_sync_bit’ undeclared (first use in this function)
  835 |      bis[num_bis] = bis_sync_bit + 1;
      |                     ^~~~~~~~~~~~
src/shared/bass.c:835:21: note: each undeclared identifier is reported only once for each function it appears in
src/shared/bass.c:855:10: error: ‘bc_addr’ undeclared (first use in this function); did you mean ‘in_addr’?
  855 |   bacpy(&bc_addr.bc_bdaddr, &bcast_src->addr);
      |          ^~~~~~~
      |          in_addr
make[1]: *** [Makefile:8399: src/shared/libshared_mainloop_la-bass.lo] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:4528: all] Error 2
##############################
Test: ScanBuild - FAIL
Desc: Run Scan Build
Output:

src/shared/ad.c:369:19: warning: Use of zero-allocated memory
                                buf[(*pos)++] = ad_type;
                                              ^
1 warning generated.
src/shared/gatt-client.c:451:21: warning: Use of memory after it is freed
        gatt_db_unregister(op->client->db, op->db_id);
                           ^~~~~~~~~~
src/shared/gatt-client.c:696:2: warning: Use of memory after it is freed
        discovery_op_complete(op, false, att_ecode);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:993:2: warning: Use of memory after it is freed
        discovery_op_complete(op, success, att_ecode);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1099:2: warning: Use of memory after it is freed
        discovery_op_complete(op, success, att_ecode);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1291:2: warning: Use of memory after it is freed
        discovery_op_complete(op, success, att_ecode);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1356:2: warning: Use of memory after it is freed
        discovery_op_complete(op, success, att_ecode);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1631:6: warning: Use of memory after it is freed
        if (read_db_hash(op)) {
            ^~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1636:2: warning: Use of memory after it is freed
        discover_all(op);
        ^~~~~~~~~~~~~~~~
src/shared/gatt-client.c:2140:6: warning: Use of memory after it is freed
        if (read_db_hash(op)) {
            ^~~~~~~~~~~~~~~~
src/shared/gatt-client.c:2148:8: warning: Use of memory after it is freed
                                                        discovery_op_ref(op),
                                                        ^~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:3236:2: warning: Use of memory after it is freed
        complete_write_long_op(req, success, 0, false);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:3258:2: warning: Use of memory after it is freed
        request_unref(req);
        ^~~~~~~~~~~~~~~~~~
12 warnings generated.
src/shared/bass.c: In function ‘bass_handle_add_src_op’:
src/shared/bass.c:835:21: error: ‘bis_sync_bit’ undeclared (first use in this function)
  835 |      bis[num_bis] = bis_sync_bit + 1;
      |                     ^~~~~~~~~~~~
src/shared/bass.c:835:21: note: each undeclared identifier is reported only once for each function it appears in
src/shared/bass.c:855:10: error: ‘bc_addr’ undeclared (first use in this function); did you mean ‘in_addr’?
  855 |   bacpy(&bc_addr.bc_bdaddr, &bcast_src->addr);
      |          ^~~~~~~
      |          in_addr
make[1]: *** [Makefile:8399: src/shared/libshared_mainloop_la-bass.lo] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:4528: all] Error 2


---
Regards,
Linux Bluetooth

Patch

diff --git a/btio/btio.c b/btio/btio.c
index b68bfb14c..179be6289 100644
--- a/btio/btio.c
+++ b/btio/btio.c
@@ -16,6 +16,7 @@ 
 
 #include <stdarg.h>
 #include <stdlib.h>
+#include <stdbool.h>
 #include <unistd.h>
 #include <errno.h>
 #include <poll.h>
@@ -71,6 +72,9 @@  struct set_opts {
 	uint16_t voice;
 	struct bt_iso_qos qos;
 	struct bt_iso_base base;
+	uint8_t bc_sid;
+	uint8_t bc_num_bis;
+	uint8_t bc_bis[ISO_MAX_NUM_BIS];
 };
 
 struct connect {
@@ -771,21 +775,47 @@  static int sco_bind(int sock, const bdaddr_t *src, GError **err)
 }
 
 static int iso_bind(int sock, const bdaddr_t *src, uint8_t src_type,
-							GError **err)
+					const bdaddr_t *dst, uint8_t dst_type,
+					uint8_t bc_sid, uint8_t num_bis,
+					uint8_t *bis, GError **err)
 {
-	struct sockaddr_iso addr;
+	struct sockaddr_iso *addr = NULL;
+	size_t addr_len;
+	int ret = 0;
 
-	memset(&addr, 0, sizeof(addr));
-	addr.iso_family = AF_BLUETOOTH;
-	bacpy(&addr.iso_bdaddr, src);
-	addr.iso_bdaddr_type = src_type;
+	if (num_bis)
+		addr_len = sizeof(*addr) + sizeof(*addr->iso_bc);
+	else
+		addr_len = sizeof(*addr);
+
+	addr = malloc(addr_len);
+
+	if (!addr)
+		return -ENOMEM;
+
+	memset(addr, 0, addr_len);
+	addr->iso_family = AF_BLUETOOTH;
+	bacpy(&addr->iso_bdaddr, src);
+	addr->iso_bdaddr_type = src_type;
 
-	if (!bind(sock, (struct sockaddr *) &addr, sizeof(addr)))
-		return 0;
+	if (num_bis) {
+		bacpy(&addr->iso_bc->bc_bdaddr, dst);
+		addr->iso_bc->bc_bdaddr_type = dst_type;
+		addr->iso_bc->bc_sid = bc_sid;
+		addr->iso_bc->bc_num_bis = num_bis;
+		memcpy(addr->iso_bc->bc_bis, bis,
+			addr->iso_bc->bc_num_bis);
+	}
+
+	if (!bind(sock, (struct sockaddr *)addr, addr_len))
+		goto done;
 
+	ret = -errno;
 	ERROR_FAILED(err, "iso_bind", errno);
 
-	return -errno;
+done:
+	free(addr);
+	return ret;
 }
 
 static int sco_connect(int sock, const bdaddr_t *dst)
@@ -980,6 +1010,16 @@  static gboolean parse_set_opts(struct set_opts *opts, GError **err,
 		case BT_IO_OPT_BASE:
 			opts->base = *va_arg(args, struct bt_iso_base *);
 			break;
+		case BT_IO_OPT_ISO_BC_SID:
+			opts->bc_sid = va_arg(args, int);
+			break;
+		case BT_IO_OPT_ISO_BC_NUM_BIS:
+			opts->bc_num_bis = va_arg(args, int);
+			break;
+		case BT_IO_OPT_ISO_BC_BIS:
+			memcpy(opts->bc_bis, va_arg(args, uint8_t *),
+					opts->bc_num_bis);
+			break;
 		case BT_IO_OPT_INVALID:
 		case BT_IO_OPT_KEY_SIZE:
 		case BT_IO_OPT_SOURCE_CHANNEL:
@@ -1305,6 +1345,9 @@  parse_opts:
 		case BT_IO_OPT_VOICE:
 		case BT_IO_OPT_QOS:
 		case BT_IO_OPT_BASE:
+		case BT_IO_OPT_ISO_BC_SID:
+		case BT_IO_OPT_ISO_BC_NUM_BIS:
+		case BT_IO_OPT_ISO_BC_BIS:
 		default:
 			g_set_error(err, BT_IO_ERROR, EINVAL,
 					"Unknown option %d", opt);
@@ -1460,6 +1503,9 @@  static gboolean rfcomm_get(int sock, GError **err, BtIOOption opt1,
 		case BT_IO_OPT_VOICE:
 		case BT_IO_OPT_QOS:
 		case BT_IO_OPT_BASE:
+		case BT_IO_OPT_ISO_BC_SID:
+		case BT_IO_OPT_ISO_BC_NUM_BIS:
+		case BT_IO_OPT_ISO_BC_BIS:
 		case BT_IO_OPT_INVALID:
 		default:
 			g_set_error(err, BT_IO_ERROR, EINVAL,
@@ -1571,6 +1617,9 @@  static gboolean sco_get(int sock, GError **err, BtIOOption opt1, va_list args)
 		case BT_IO_OPT_VOICE:
 		case BT_IO_OPT_QOS:
 		case BT_IO_OPT_BASE:
+		case BT_IO_OPT_ISO_BC_SID:
+		case BT_IO_OPT_ISO_BC_NUM_BIS:
+		case BT_IO_OPT_ISO_BC_BIS:
 		case BT_IO_OPT_INVALID:
 		default:
 			g_set_error(err, BT_IO_ERROR, EINVAL,
@@ -1660,6 +1709,9 @@  static gboolean iso_get(int sock, GError **err, BtIOOption opt1, va_list args)
 		case BT_IO_OPT_FLUSHABLE:
 		case BT_IO_OPT_PRIORITY:
 		case BT_IO_OPT_VOICE:
+		case BT_IO_OPT_ISO_BC_SID:
+		case BT_IO_OPT_ISO_BC_NUM_BIS:
+		case BT_IO_OPT_ISO_BC_BIS:
 		case BT_IO_OPT_INVALID:
 		default:
 			g_set_error(err, BT_IO_ERROR, EINVAL,
@@ -1836,7 +1888,11 @@  static GIOChannel *create_io(gboolean server, struct set_opts *opts,
 			ERROR_FAILED(err, "socket(SEQPACKET, ISO)", errno);
 			return NULL;
 		}
-		if (iso_bind(sock, &opts->src, opts->src_type, err) < 0)
+
+		if (iso_bind(sock, &opts->src, opts->src_type,
+				 &opts->dst, opts->dst_type,
+				 opts->bc_sid, opts->bc_num_bis,
+				 opts->bc_bis, err) < 0)
 			goto failed;
 		if (!iso_set_qos(sock, &opts->qos, err))
 			goto failed;
diff --git a/btio/btio.h b/btio/btio.h
index e9a8a01a3..642af2e22 100644
--- a/btio/btio.h
+++ b/btio/btio.h
@@ -46,7 +46,10 @@  typedef enum {
 	BT_IO_OPT_VOICE,
 	BT_IO_OPT_PHY,
 	BT_IO_OPT_QOS,
-	BT_IO_OPT_BASE
+	BT_IO_OPT_BASE,
+	BT_IO_OPT_ISO_BC_SID,
+	BT_IO_OPT_ISO_BC_NUM_BIS,
+	BT_IO_OPT_ISO_BC_BIS,
 } BtIOOption;
 
 typedef enum {