Hi Marek,
On Sun, Mar 7, 2021 at 12:26 PM Marek Behún <marek.be...@nic.cz> wrote:
>
> The api_public.h header file undefined macro CONFIG_SYS_64BIT_LBA.
>
> But api/api_storage.c includes this header before including part.h,
> causing the type of lbaint_t and subsequently the type signature of
> blk_dread() and blk_dwrite() functions to change from the rest of U-Boot
> (if CONFIG_SYS_64BIT_LBA is defined for the board).
>
> This is of course wrong, because the call to blk_dread() / blk_dwrite()
> will recieve mangled arguments.
typo: receive
>
> Fix this by removing the undef of macro CONFIG_SYS_64BIT_LBA and instead
> make the immediate code do what it would do as if the macro was not
> defined.
>
> Add a FIXME to whoever is maintaining this code.
>
> CI managed to trigger this bug when compiling for lsxhl_defconfig, which
> has CONFIG_API selected. The compiler complained about blk_dwrite() and
> blk_dread() not matching original declarations:
>
> include/blk.h:280:15: warning: type of ‘blk_dwrite’ does not match
> original declaration
> [-Wlto-type-mismatch]
> 280 | unsigned long blk_dwrite(struct blk_desc *block_dev, lbaint_t st
> | ^
> drivers/block/blk-uclass.c:456:15: note: type mismatch in parameter 2
> 456 | unsigned long blk_dwrite(struct blk_desc *block_dev, lbaint_t st
> | ^
>
> Signed-off-by: Marek Behún <marek.be...@nic.cz>
> ---
> include/api_public.h | 23 ++++++++++++++++++-----
> 1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/include/api_public.h b/include/api_public.h
> index def103ce22..5a4465ea89 100644
> --- a/include/api_public.h
> +++ b/include/api_public.h
> @@ -70,12 +70,25 @@ struct sys_info {
> int mr_no; /* number of memory regions */
> };
>
> -#undef CONFIG_SYS_64BIT_LBA
> -#ifdef CONFIG_SYS_64BIT_LBA
> -typedef u_int64_t lbasize_t;
> -#else
> +/*
> + * FIXME: Previously this code was:
> + *
> + * #undef CONFIG_SYS_64BIT_LBA
> + * #ifdef CONFIG_SYS_64BIT_LBA
> + * typedef u_int64_t lbasize_t;
> + * #else
> + * typedef unsigned long lbasize_t;
> + * #endif
> + *
> + * But we cannot just undefine CONFIG_SYS_64BIT_LBA, because then in
> + * api/api_storage.c the type signature of lbaint_t will be different if
> + * CONFIG_SYS_64BIT_LBA is enabled for the board, which can result in various
> + * bugs.
> + * So simply define lbasize_t as an unsigned long, since this was what was
> done
> + * anyway for at least 13 years, but don't undefine CONFIG_SYS_64BIT_LBA.
> + */
> typedef unsigned long lbasize_t;
Does "typedef lbaint_t labsize_t" help?
> -#endif
> +
> typedef unsigned long lbastart_t;
Regards,
Bin
