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

[Pierre-Louis Bossart]

On 1/22/18 6:15 AM, Liam Girdwood wrote:
> On Fri, 2018-01-19 at 16:33 -0600, Pierre-Louis Bossart wrote:
>> 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)
> 
> probably best making this size_t offset, void *src, size_t bytes so it
> becomes more portable (and also follows the memcpy() convention a
> little more).

Yeah, that's what I was thinking initially but then the msg->rx_data is 
uint8_t so what generates fewer typecasts
Will use void and size_t

> 
> Liam
>