Re: [U-Boot] [PATCH v3] arm: socfpga: mailbox: Fix off-by-one error on command length checking

On 4/19/19 10:52 PM, Simon Goldschmidt wrote:

Marek Vasut <marex@denx.de mailto:marex@denx.de> schrieb am Fr., 19. Apr. 2019, 11:29:

On 4/19/19 8:17 AM, Ley Foon Tan wrote:
> A mailbox command contains 1-DWORD header + arguments. The "len"
variable
> only contains the length of the arguments, but not the 1-DWORD header.
> Include the length of header when checking the ring buffer space to
> prevent off-by-one error.

How long is a DWORD ? Windows API (which we have nothing to do with)
defines that as 32bit type, "typedef unsigned long DWORD;", see [1].
But the patch below fixes an off-by-one error , not off by four error ?

As all the macros for that mailbox seem to do u32 index access only, I'd be ok with the commit message if it didn't use the term 'DWORD'...

Does that mean MBOX_CMD_BUFFER_SIZE is not in byte units, but in u32 units ?

Regards, Simon

[1]
https://docs.microsoft.com/en-us/windows/desktop/winprog/windows-data-types

> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com
<mailto:ley.foon.tan@intel.com>>
> Signed-off-by: Chee Hong Ang <chee.hong.ang@intel.com
<mailto:chee.hong.ang@intel.com>>
> ---
> v2->v3:
> - Update commit description.
> ---
>  arch/arm/mach-socfpga/mailbox_s10.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-socfpga/mailbox_s10.c
b/arch/arm/mach-socfpga/mailbox_s10.c
> index 3c33223936..8363c93e90 100644
> --- a/arch/arm/mach-socfpga/mailbox_s10.c
> +++ b/arch/arm/mach-socfpga/mailbox_s10.c
> @@ -59,7 +59,7 @@ static __always_inline int
mbox_fill_cmd_circular_buff(u32 header, u32 len,
>        */
>       if (((cin + 1) % MBOX_CMD_BUFFER_SIZE) == cout ||
>           ((MBOX_CMD_BUFFER_SIZE - cin + cout - 1) %
> -          MBOX_CMD_BUFFER_SIZE) < len)
> +          MBOX_CMD_BUFFER_SIZE) < (len + 1))
>               return -ENOMEM;
> 
>       /* write header to circular buffer */
>


-- 
Best regards,
Marek Vasut