[Sound-open-firmware] [RFC PATCH] mailbox: move multi-line macros to inline functions

Pierre-Louis Bossart pierre-louis.bossart at linux.intel.com
Fri Jan 19 23:33:12 CET 2018


Detected with Coverity:

In do_notify: The indentation of this code suggests it is nested when
it is not. (CWE-483)

multi_stmt_macro: The macro on this line expands into multiple
statements, only the first of which is nested within the preceding
parent while the rest are not.

	if (msg->rx_size && msg->rx_size < SOF_IPC_MSG_MAX_SIZE)
		mailbox_dspbox_read(msg->rx_data, 0, msg->rx_size);

Move mailbox macros to inline functions to remove the issue, keep
indentation the same and add typecasts as needed

FIXME:
a. is type uint8_t * ok for data?
b. is uint32_t ok for offset (renamed from dest since it was
not a pointer)
c. This code is full of integer/pointer conversions
which will have to be cleaned-up for MISRA compliance

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart at linux.intel.com>
---
 src/include/reef/mailbox.h       | 44 ++++++++++++++++++++++++++--------------
 src/ipc/byt-ipc.c                |  2 +-
 src/ipc/intel-ipc.c              | 23 +++++++++++----------
 src/platform/baytrail/platform.c |  2 +-
 4 files changed, 43 insertions(+), 28 deletions(-)

diff --git a/src/include/reef/mailbox.h b/src/include/reef/mailbox.h
index c0580a9..c3976a3 100644
--- a/src/include/reef/mailbox.h
+++ b/src/include/reef/mailbox.h
@@ -34,6 +34,7 @@
 #include <platform/mailbox.h>
 #include <arch/cache.h>
 #include <stdint.h>
+#include <reef/reef.h>
 
 /* 4k should be enough for everyone ..... */
 #define IPC_MAX_MAILBOX_BYTES 0x1000
@@ -62,20 +63,33 @@
 #define mailbox_get_debug_size() \
 	MAILBOX_DEBUG_SIZE
 
-#define mailbox_dspbox_write(dest, src, bytes) \
-	rmemcpy((void*)(MAILBOX_DSPBOX_BASE + dest), src, bytes); \
-	dcache_writeback_region((void*)(MAILBOX_DSPBOX_BASE + dest), bytes);
-
-#define mailbox_dspbox_read(dest, src, bytes) \
-	dcache_invalidate_region((void*)(MAILBOX_DSPBOX_BASE + src), bytes); \
-	rmemcpy(dest, (void*)(MAILBOX_DSPBOX_BASE + src), bytes);
-
-#define mailbox_hostbox_write(dest, src, bytes) \
-	rmemcpy((void*)(MAILBOX_HOSTBOX_BASE + dest), src, bytes); \
-	dcache_writeback_region((void*)(MAILBOX_HOSTBOX_BASE + dest), bytes);
-
-#define mailbox_hostbox_read(dest, src, bytes) \
-	dcache_invalidate_region((void*)(MAILBOX_HOSTBOX_BASE + src), bytes); \
-	rmemcpy(dest, (void*)(MAILBOX_HOSTBOX_BASE + src), bytes);
+static inline
+void mailbox_dspbox_write(uint32_t offset, uint8_t *src, uint32_t bytes)
+{
+	rmemcpy((void *)(MAILBOX_DSPBOX_BASE + offset), src, bytes);
+	dcache_writeback_region((void *)(MAILBOX_DSPBOX_BASE + offset), bytes);
+}
+
+static inline
+void mailbox_dspbox_read(uint8_t *dest, uint32_t offset, uint32_t bytes)
+{
+	dcache_invalidate_region((void *)(MAILBOX_DSPBOX_BASE + offset), bytes);
+	rmemcpy(dest, (void *)(MAILBOX_DSPBOX_BASE + offset), bytes);
+}
+
+static inline
+void mailbox_hostbox_write(uint32_t offset, uint8_t *src, uint32_t bytes)
+{
+	rmemcpy((void *)(MAILBOX_HOSTBOX_BASE + offset), src, bytes);
+	dcache_writeback_region((void *)(MAILBOX_HOSTBOX_BASE + offset), bytes);
+}
+
+static inline
+void mailbox_hostbox_read(uint8_t *dest, uint32_t offset, uint32_t bytes)
+{
+	dcache_invalidate_region((void *)(MAILBOX_HOSTBOX_BASE + offset),
+				 bytes);
+	rmemcpy(dest, (void *)(MAILBOX_HOSTBOX_BASE + offset), bytes);
+}
 
 #endif
diff --git a/src/ipc/byt-ipc.c b/src/ipc/byt-ipc.c
index 8897bb9..029d3a6 100644
--- a/src/ipc/byt-ipc.c
+++ b/src/ipc/byt-ipc.c
@@ -140,7 +140,7 @@ void ipc_platform_do_cmd(struct ipc *ipc)
 	/* send std error/ok reply */
 	reply.hdr.cmd = SOF_IPC_GLB_REPLY;
 	reply.hdr.size = sizeof(reply);
-	mailbox_hostbox_write(0, &reply, sizeof(reply));
+	mailbox_hostbox_write(0, (uint8_t *)&reply, sizeof(reply));
 
 done:
 	ipc->host_pending = 0;
diff --git a/src/ipc/intel-ipc.c b/src/ipc/intel-ipc.c
index 5a98e1f..99e0a90 100644
--- a/src/ipc/intel-ipc.c
+++ b/src/ipc/intel-ipc.c
@@ -69,7 +69,7 @@ static inline struct sof_ipc_hdr *mailbox_validate(void)
 	struct sof_ipc_hdr *hdr = _ipc->comp_data;
 
 	/* read component values from the inbox */
-	mailbox_hostbox_read(hdr, 0, sizeof(*hdr));
+	mailbox_hostbox_read((uint8_t *)hdr, 0, sizeof(*hdr));
 
 	/* validate component header */
 	if (hdr->size > SOF_IPC_MSG_MAX_SIZE) {
@@ -78,7 +78,8 @@ static inline struct sof_ipc_hdr *mailbox_validate(void)
 	}
 
 	/* read rest of component data */
-	mailbox_hostbox_read(hdr + 1, sizeof(*hdr), hdr->size - sizeof(*hdr));
+	mailbox_hostbox_read((uint8_t *)(hdr + 1), sizeof(*hdr),
+			     hdr->size - sizeof(*hdr));
 	return hdr;
 }
 
@@ -297,7 +298,7 @@ static int ipc_stream_pcm_params(uint32_t stream)
 	reply.rhdr.error = 0;
 	reply.comp_id = pcm_params->comp_id;
 	reply.posn_offset = 0; /* TODO: set this up for mmaped components */
-	mailbox_hostbox_write(0, &reply, sizeof(reply));
+	mailbox_hostbox_write(0, (uint8_t *)&reply, sizeof(reply));
 	return 1;
 
 error:
@@ -360,7 +361,7 @@ static int ipc_stream_position(uint32_t header)
 	pipeline_get_timestamp(pcm_dev->cd->pipeline, pcm_dev->cd, &posn);
 
 	/* copy positions to outbox */
-	mailbox_hostbox_write(0, &posn, sizeof(posn));
+	mailbox_hostbox_write(0, (uint8_t *)&posn, sizeof(posn));
 	return 1;
 }
 
@@ -521,7 +522,7 @@ static int ipc_pm_context_size(uint32_t header)
 	/* TODO: calculate the context and size of host buffers required */
 
 	/* write the context to the host driver */
-	mailbox_hostbox_write(0, &pm_ctx, sizeof(pm_ctx));
+	mailbox_hostbox_write(0, (uint8_t *)&pm_ctx, sizeof(pm_ctx));
 	return 1;
 }
 
@@ -554,7 +555,7 @@ static int ipc_pm_context_save(uint32_t header)
 	//reply.entries_no = 0;
 
 	/* write the context to the host driver */
-	mailbox_hostbox_write(0, pm_ctx, sizeof(*pm_ctx));
+	mailbox_hostbox_write(0, (uint8_t *)pm_ctx, sizeof(*pm_ctx));
 
 	//iipc->pm_prepare_D3 = 1;
 
@@ -632,7 +633,7 @@ static int ipc_dma_trace_config(uint32_t header)
 	reply.hdr.size = sizeof(reply);
 	reply.hdr.cmd = header;
 	reply.error = 0;
-	mailbox_hostbox_write(0, &reply, sizeof(reply));
+	mailbox_hostbox_write(0, (uint8_t *)&reply, sizeof(reply));
 	return 0;
 
 error:
@@ -702,7 +703,7 @@ static int ipc_comp_value(uint32_t header, uint32_t cmd)
 	}
 
 	/* write component values to the outbox */
-	mailbox_hostbox_write(0, data, data->rhdr.hdr.size);
+	mailbox_hostbox_write(0, (uint8_t *)data, data->rhdr.hdr.size);
 	return 1;
 }
 
@@ -746,7 +747,7 @@ static int ipc_glb_tplg_comp_new(uint32_t header)
 	reply.rhdr.hdr.cmd = header;
 	reply.rhdr.error = 0;
 	reply.offset = 0; /* TODO: set this up for mmaped components */
-	mailbox_hostbox_write(0, &reply, sizeof(reply));
+	mailbox_hostbox_write(0, (uint8_t *)&reply, sizeof(reply));
 	return 1;
 }
 
@@ -769,7 +770,7 @@ static int ipc_glb_tplg_buffer_new(uint32_t header)
 	reply.rhdr.hdr.cmd = header;
 	reply.rhdr.error = 0;
 	reply.offset = 0; /* TODO: set this up for mmaped components */
-	mailbox_hostbox_write(0, &reply, sizeof(reply));
+	mailbox_hostbox_write(0, (uint8_t *)&reply, sizeof(reply));
 	return 1;
 }
 
@@ -792,7 +793,7 @@ static int ipc_glb_tplg_pipe_new(uint32_t header)
 	reply.rhdr.hdr.cmd = header;
 	reply.rhdr.error = 0;
 	reply.offset = 0; /* TODO: set this up for mmaped components */
-	mailbox_hostbox_write(0, &reply, sizeof(reply));
+	mailbox_hostbox_write(0, (uint8_t *)&reply, sizeof(reply));
 	return 1;
 }
 
diff --git a/src/platform/baytrail/platform.c b/src/platform/baytrail/platform.c
index 3114ac1..c5c9108 100644
--- a/src/platform/baytrail/platform.c
+++ b/src/platform/baytrail/platform.c
@@ -92,7 +92,7 @@ int platform_boot_complete(uint32_t boot_message)
 {
 	uint64_t outbox = MAILBOX_HOST_OFFSET >> 3;
 
-	mailbox_dspbox_write(0, &ready, sizeof(ready));
+	mailbox_dspbox_write(0, (uint8_t *)&ready, sizeof(ready));
 
 	/* now interrupt host to tell it we are done booting */
 	shim_write(SHIM_IPCDL, SOF_IPC_FW_READY | outbox);
-- 
2.14.1



More information about the Sound-open-firmware mailing list