mgmt: mcumgr: smp: Convert functions to function pointer array

Converts the list of functions for registering an MCUmgr SMP
transport from function arguments to a single list which contains
the function pointers.

Signed-off-by: Jamie McCrae <[email protected]>

Author: nordicjm

include/zephyr/mgmt/mcumgr/transport/smp.h

@@ -1,6 +1,6 @@
 /*
  * Copyright Runtime.io 2018. All rights reserved.
- * Copyright (c) 2022 Nordic Semiconductor ASA
+ * Copyright (c) 2022-2023 Nordic Semiconductor ASA
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -35,8 +35,6 @@ struct net_buf;
  * @return                      0 on success, #mcumgr_err_t code on failure.
  */
 typedef int (*smp_transport_out_fn)(struct net_buf *nb);
-/* use smp_transport_out_fn instead */
-typedef int zephyr_smp_transport_out_fn(struct net_buf *nb);
 
 /** @typedef smp_transport_get_mtu_fn
  * @brief SMP MTU query callback for transport
@@ -52,8 +50,6 @@ typedef int zephyr_smp_transport_out_fn(struct net_buf *nb);
  *                              0 if transmission is currently not possible.
  */
 typedef uint16_t (*smp_transport_get_mtu_fn)(const struct net_buf *nb);
-/* use smp_transport_get_mtu_fn instead */
-typedef uint16_t zephyr_smp_transport_get_mtu_fn(const struct net_buf *nb);
 
 /** @typedef smp_transport_ud_copy_fn
  * @brief SMP copy user_data callback
@@ -70,9 +66,6 @@ typedef uint16_t zephyr_smp_transport_get_mtu_fn(const struct net_buf *nb);
  */
 typedef int (*smp_transport_ud_copy_fn)(struct net_buf *dst,
 					const struct net_buf *src);
-/* use smp_transport_ud_copy_fn instead */
-typedef int zephyr_smp_transport_ud_copy_fn(struct net_buf *dst,
-					    const struct net_buf *src);
 
 /** @typedef smp_transport_ud_free_fn
  * @brief SMP free user_data callback
@@ -84,8 +77,6 @@ typedef int zephyr_smp_transport_ud_copy_fn(struct net_buf *dst,
  * @param ud                    Contains a user_data pointer to be freed.
  */
 typedef void (*smp_transport_ud_free_fn)(void *ud);
-/* use smp_transport_ud_free_fn instead */
-typedef void zephyr_smp_transport_ud_free_fn(void *ud);
 
 /** @typedef smp_transport_query_valid_check_fn
  * @brief Function for checking if queued data is still valid.
@@ -101,42 +92,38 @@ typedef void zephyr_smp_transport_ud_free_fn(void *ud);
 typedef bool (*smp_transport_query_valid_check_fn)(struct net_buf *nb, void *arg);
 
 /**
- * @brief SMP transport object for sending SMP responses.
+ * @brief Function pointers of SMP transport functions, if a handler is NULL then it is not
+ * supported/implemented.
  */
-struct smp_transport {
-	/* Must be the first member. */
-	struct k_work work;
-
-	/* FIFO containing incoming requests to be processed. */
-	struct k_fifo fifo;
-
+struct smp_transport_api_t {
+	/** Transport's send function. */
 	smp_transport_out_fn output;
+
+	/** Transport's get-MTU function. */
 	smp_transport_get_mtu_fn get_mtu;
+
+	/** Transport buffer user_data copy function. */
 	smp_transport_ud_copy_fn ud_copy;
+
+	/** Transport buffer user_data free function. */
 	smp_transport_ud_free_fn ud_free;
-	smp_transport_query_valid_check_fn query_valid_check;
 
-#ifdef CONFIG_MCUMGR_TRANSPORT_REASSEMBLY
-	/* Packet reassembly internal data, API access only */
-	struct {
-		struct net_buf *current;	/* net_buf used for reassembly */
-		uint16_t expected;		/* expected bytes to come */
-	} __reassembly;
-#endif
+	/** Transport's check function for if a query is valid. */
+	smp_transport_query_valid_check_fn query_valid_check;
 };
 
-/* Deprecated, use smp_transport instead */
-struct zephyr_smp_transport {
+/**
+ * @brief SMP transport object for sending SMP responses.
+ */
+struct smp_transport {
 	/* Must be the first member. */
-	struct k_work zst_work;
+	struct k_work work;
 
 	/* FIFO containing incoming requests to be processed. */
-	struct k_fifo zst_fifo;
+	struct k_fifo fifo;
 
-	smp_transport_out_fn zst_output;
-	smp_transport_get_mtu_fn zst_get_mtu;
-	smp_transport_ud_copy_fn zst_ud_copy;
-	smp_transport_ud_free_fn zst_ud_free;
+	/* Function pointers */
+	struct smp_transport_api_t functions;
 
 #ifdef CONFIG_MCUMGR_TRANSPORT_REASSEMBLY
 	/* Packet reassembly internal data, API access only */
@@ -150,33 +137,12 @@ struct zephyr_smp_transport {
 /**
  * @brief Initializes a Zephyr SMP transport object.
  *
- * @param smpt				The transport to construct.
- * @param output_func			The transport's send function.
- * @param get_mtu_func			The transport's get-MTU function.
- * @param ud_copy_func			The transport buffer user_data copy function.
- * @param ud_free_func			The transport buffer user_data free function.
- * @param query_valid_check_func	The transport's check function for if a query is valid.
+ * @param smpt	The transport to construct.
+ *
+ * @return	0 If successful
+ * @return	Negative errno code if failure.
  */
-void smp_transport_init(struct smp_transport *smpt,
-			smp_transport_out_fn output_func,
-			smp_transport_get_mtu_fn get_mtu_func,
-			smp_transport_ud_copy_fn ud_copy_func,
-			smp_transport_ud_free_fn ud_free_func,
-			smp_transport_query_valid_check_fn query_valid_check_func);
-
-__deprecated static inline
-void zephyr_smp_transport_init(struct zephyr_smp_transport *smpt,
-			       zephyr_smp_transport_out_fn *output_func,
-			       zephyr_smp_transport_get_mtu_fn *get_mtu_func,
-			       zephyr_smp_transport_ud_copy_fn *ud_copy_func,
-			       zephyr_smp_transport_ud_free_fn *ud_free_func)
-{
-	smp_transport_init((struct smp_transport *)smpt,
-			   (smp_transport_out_fn)output_func,
-			   (smp_transport_get_mtu_fn)get_mtu_func,
-			   (smp_transport_ud_copy_fn)ud_copy_func,
-			   (smp_transport_ud_free_fn)ud_free_func, NULL);
-}
+int smp_transport_init(struct smp_transport *smpt);
 
 /**
  * @brief	Used to remove queued requests for an SMP transport that are no longer valid. A

subsys/mgmt/mcumgr/smp/src/smp.c

@@ -334,7 +334,7 @@ static void smp_on_err(struct smp_streamer *streamer, const struct smp_hdr *req_
 	/* Build and transmit the error response. */
 	rc = smp_build_err_rsp(streamer, req_hdr, status, rsn);
 	if (rc == 0) {
-		streamer->smpt->output(rsp);
+		streamer->smpt->functions.output(rsp);
 		rsp = NULL;
 	}
 
@@ -415,7 +415,7 @@ int smp_process_request_packet(struct smp_streamer *streamer, void *vreq)
 		}
 
 		/* Send the response. */
-		rc = streamer->smpt->output(rsp);
+		rc = streamer->smpt->functions.output(rsp);
 		rsp = NULL;
 		if (rc != 0) {
 			break;

subsys/mgmt/mcumgr/transport/src/smp.c

@@ -5,6 +5,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+#include <assert.h>
 #include <zephyr/kernel.h>
 #include <zephyr/device.h>
 #include <zephyr/net/buf.h>
@@ -72,8 +73,8 @@ void *smp_alloc_rsp(const void *req, void *arg)
 		return NULL;
 	}
 
-	if (smpt->ud_copy) {
-		smpt->ud_copy(rsp_nb, req_nb);
+	if (smpt->functions.ud_copy) {
+		smpt->functions.ud_copy(rsp_nb, req_nb);
 	} else {
 		memcpy(net_buf_user_data(rsp_nb),
 		       net_buf_user_data((void *)req_nb),
@@ -91,8 +92,8 @@ void smp_free_buf(void *buf, void *arg)
 		return;
 	}
 
-	if (smpt->ud_free) {
-		smpt->ud_free(net_buf_user_data((struct net_buf *)buf));
+	if (smpt->functions.ud_free) {
+		smpt->functions.ud_free(net_buf_user_data((struct net_buf *)buf));
 	}
 
 	smp_packet_free(buf);
@@ -135,28 +136,23 @@ smp_handle_reqs(struct k_work *work)
 	}
 }
 
-void
-smp_transport_init(struct smp_transport *smpt,
-		   smp_transport_out_fn output_func,
-		   smp_transport_get_mtu_fn get_mtu_func,
-		   smp_transport_ud_copy_fn ud_copy_func,
-		   smp_transport_ud_free_fn ud_free_func,
-		   smp_transport_query_valid_check_fn query_valid_check_func)
+int smp_transport_init(struct smp_transport *smpt)
 {
-	*smpt = (struct smp_transport) {
-		.output = output_func,
-		.get_mtu = get_mtu_func,
-		.ud_copy = ud_copy_func,
-		.ud_free = ud_free_func,
-		.query_valid_check = query_valid_check_func,
-	};
+	__ASSERT((smpt->functions.output != NULL),
+		 "Required transport output function pointer cannot be NULL");
+
+	if (smpt->functions.output == NULL) {
+		return -EINVAL;
+	}
 
 #ifdef CONFIG_MCUMGR_TRANSPORT_REASSEMBLY
 	smp_reassembly_init(smpt);
 #endif
 
 	k_work_init(&smpt->work, smp_handle_reqs);
 	k_fifo_init(&smpt->fifo);
+
+	return 0;
 }
 
 /**
@@ -180,7 +176,7 @@ void smp_rx_remove_invalid(struct smp_transport *zst, void *arg)
 	struct net_buf *nb;
 	struct k_fifo temp_fifo;
 
-	if (zst->query_valid_check == NULL) {
+	if (zst->functions.query_valid_check == NULL) {
 		/* No check check function registered, abort check */
 		return;
 	}
@@ -196,7 +192,7 @@ void smp_rx_remove_invalid(struct smp_transport *zst, void *arg)
 	k_fifo_init(&temp_fifo);
 
 	while ((nb = net_buf_get(&zst->fifo, K_NO_WAIT)) != NULL) {
-		if (!zst->query_valid_check(nb, arg)) {
+		if (!zst->functions.query_valid_check(nb, arg)) {
 			smp_free_buf(nb, zst);
 		} else {
 			net_buf_put(&temp_fifo, nb);

subsys/mgmt/mcumgr/transport/src/smp_bt.c

@@ -653,11 +653,17 @@ static void smp_bt_setup(void)
 		++i;
 	}
 
-	smp_transport_init(&smp_bt_transport, smp_bt_tx_pkt,
-			   smp_bt_get_mtu, smp_bt_ud_copy,
-			   smp_bt_ud_free, smp_bt_query_valid_check);
+	smp_bt_transport.functions.output = smp_bt_tx_pkt;
+	smp_bt_transport.functions.get_mtu = smp_bt_get_mtu;
+	smp_bt_transport.functions.ud_copy = smp_bt_ud_copy;
+	smp_bt_transport.functions.ud_free = smp_bt_ud_free;
+	smp_bt_transport.functions.query_valid_check = smp_bt_query_valid_check;
 
-	rc = smp_bt_register();
+	rc = smp_transport_init(&smp_bt_transport);
+
+	if (rc == 0) {
+		rc = smp_bt_register();
+	}
 
 	if (rc != 0) {
 		LOG_ERR("Bluetooth SMP transport register failed (err %d)", rc);

subsys/mgmt/mcumgr/transport/src/smp_dummy.c

@@ -187,17 +187,24 @@ static int smp_dummy_tx_pkt_int(struct net_buf *nb)
 
 static int smp_dummy_init(void)
 {
+	int rc;
 
 	k_sem_init(&smp_data_ready_sem, 0, 1);
 
-	smp_transport_init(&smp_dummy_transport, smp_dummy_tx_pkt_int,
-			   smp_dummy_get_mtu, NULL, NULL, NULL);
+	smp_dummy_transport.functions.output = smp_dummy_tx_pkt_int;
+	smp_dummy_transport.functions.get_mtu = smp_dummy_get_mtu;
+
+	rc = smp_transport_init(&smp_dummy_transport);
+
+	if (rc != 0) {
+		return rc;
+	}
+
 	dummy_mgumgr_recv_cb = smp_dummy_rx_frag;
 
 	return 0;
 }
 
-
 static struct uart_mcumgr_rx_buf *dummy_mcumgr_alloc_rx_buf(void)
 {
 	struct uart_mcumgr_rx_buf *rx_buf;

subsys/mgmt/mcumgr/transport/src/smp_shell.c

@@ -235,8 +235,12 @@ static int smp_shell_tx_pkt(struct net_buf *nb)
 
 int smp_shell_init(void)
 {
-	smp_transport_init(&smp_shell_transport, smp_shell_tx_pkt,
-			   smp_shell_get_mtu, NULL, NULL, NULL);
+	int rc;
 
-	return 0;
+	smp_shell_transport.functions.output = smp_shell_tx_pkt;
+	smp_shell_transport.functions.get_mtu = smp_shell_get_mtu;
+
+	rc = smp_transport_init(&smp_shell_transport);
+
+	return rc;
 }

subsys/mgmt/mcumgr/transport/src/smp_uart.c

@@ -92,12 +92,18 @@ static int smp_uart_tx_pkt(struct net_buf *nb)
 
 static int smp_uart_init(void)
 {
+	int rc;
+
+	smp_uart_transport.functions.output = smp_uart_tx_pkt;
+	smp_uart_transport.functions.get_mtu = smp_uart_get_mtu;
 
-	smp_transport_init(&smp_uart_transport, smp_uart_tx_pkt,
-			   smp_uart_get_mtu, NULL, NULL, NULL);
-	uart_mcumgr_register(smp_uart_rx_frag);
+	rc = smp_transport_init(&smp_uart_transport);
 
-	return 0;
+	if (rc == 0) {
+		uart_mcumgr_register(smp_uart_rx_frag);
+	}
+
+	return rc;
 }
 
 SYS_INIT(smp_uart_init, APPLICATION, CONFIG_APPLICATION_INIT_PRIORITY);

subsys/mgmt/mcumgr/transport/src/smp_udp.c

@@ -174,20 +174,25 @@ static void smp_udp_receive_thread(void *p1, void *p2, void *p3)
 
 static int smp_udp_init(void)
 {
+	int rc;
 
 #ifdef CONFIG_MCUMGR_TRANSPORT_UDP_IPV4
-	smp_transport_init(&configs.ipv4.smp_transport,
-			   smp_udp4_tx, smp_udp_get_mtu,
-			   smp_udp_ud_copy, NULL, NULL);
+	configs.ipv4.smp_transport.functions.output = smp_udp4_tx;
+	configs.ipv4.smp_transport.functions.get_mtu = smp_udp_get_mtu;
+	configs.ipv4.smp_transport.functions.ud_copy = smp_udp_ud_copy;
+
+	rc = smp_transport_init(&configs.ipv4.smp_transport);
 #endif
 
 #ifdef CONFIG_MCUMGR_TRANSPORT_UDP_IPV6
-	smp_transport_init(&configs.ipv6.smp_transport,
-			   smp_udp6_tx, smp_udp_get_mtu,
-			   smp_udp_ud_copy, NULL, NULL);
+	configs.ipv6.smp_transport.functions.output = smp_udp6_tx;
+	configs.ipv6.smp_transport.functions.get_mtu = smp_udp_get_mtu;
+	configs.ipv6.smp_transport.functions.ud_copy = smp_udp_ud_copy;
+
+	rc = smp_transport_init(&configs.ipv6.smp_transport);
 #endif
 
-	return MGMT_ERR_EOK;
+	return rc;
 }
 
 static int create_socket(struct sockaddr *addr, const char *proto)