[PATCH V8 14/14] common: Replace #ifdef and #if with if's

[Devarsh Thakkar]

Hi Nikhil,

Thanks for the patch.
On 19/04/23 11:40, Nikhil M Jain wrote:
> Avoid using preprocessor compilation directives and instead use simple
> logical expressions for better readability since compiler will anyway
> optimize out the respective code block if condition is not satisfied.
> 
> Signed-off-by: Nikhil M Jain <n-jain1 at ti.com>
> ---
> V8:
> - Update as per review comments.
> - Call bmp_display only when CONFIG_BMP is defined.
> 
> V7(patch introduced):
> - Replace #ifdef and #if with if's.
> 
>  common/bmp.c     | 14 +++++---------
>  common/splash.c  | 14 +++++++-------
>  include/splash.h | 11 -----------
>  3 files changed, 12 insertions(+), 27 deletions(-)
> 
> diff --git a/common/bmp.c b/common/bmp.c
> index ad91351f19..57764f3653 100644
> --- a/common/bmp.c
> +++ b/common/bmp.c
> @@ -31,9 +31,9 @@
>   * by the caller after use.
>   *
>   * Returns NULL if decompression failed, or if the decompressed data
> - * didn't contain a valid BMP signature.
> + * didn't contain a valid BMP signature or decompression is not enabled in
> + * Kconfig.
>   */
> -#if CONFIG_IS_ENABLED(VIDEO_BMP_GZIP)
>  struct bmp_image *gunzip_bmp(unsigned long addr, unsigned long *lenp,
>  			     void **alloc_addr)
>  {
> @@ -41,6 +41,9 @@ struct bmp_image *gunzip_bmp(unsigned long addr, unsigned long *lenp,
>  	unsigned long len;
>  	struct bmp_image *bmp;
>  
> +	if (!CONFIG_IS_ENABLED(VIDEO_BMP_GZIP))
> +		return NULL;
> +
>  	/*
>  	 * Decompress bmp image
>  	 */
> @@ -77,13 +80,6 @@ struct bmp_image *gunzip_bmp(unsigned long addr, unsigned long *lenp,
>  	*alloc_addr = dst;
>  	return bmp;
>  }
> -#else
> -struct bmp_image *gunzip_bmp(unsigned long addr, unsigned long *lenp,
> -			     void **alloc_addr)
> -{
> -	return NULL;
> -}
> -#endif
>  
>  #ifdef CONFIG_NEEDS_MANUAL_RELOC
>  void bmp_reloc(void)
> diff --git a/common/splash.c b/common/splash.c
> index a4e68b7042..2a6d83d695 100644
> --- a/common/splash.c
> +++ b/common/splash.c
> @@ -96,12 +96,11 @@ __weak int splash_screen_prepare(void)
>  	return splash_video_logo_load();
>  }
>  
> -#if CONFIG_IS_ENABLED(SPLASH_SCREEN_ALIGN)
>  void splash_get_pos(int *x, int *y)
>  {
>  	char *s = env_get("splashpos");
>  
> -	if (!s)
> +	if (!CONFIG_IS_ENABLED(SPLASH_SCREEN_ALIGN) || !s)
>  		return;
>  
>  	if (s[0] == 'm')
> @@ -117,7 +116,6 @@ void splash_get_pos(int *x, int *y)
>  			*y = simple_strtol(s + 1, NULL, 0);
>  	}
>  }
> -#endif /* CONFIG_SPLASH_SCREEN_ALIGN */
>  
>  #if CONFIG_IS_ENABLED(VIDEO) && !CONFIG_IS_ENABLED(HIDE_LOGO_VERSION)
>  
> @@ -159,13 +157,13 @@ void splash_display_banner(void)
>   * Common function to show a splash image if env("splashimage") is set.
>   * For additional details please refer to doc/README.splashprepare.
>   */
> -#if CONFIG_IS_ENABLED(SPLASH_SCREEN) && CONFIG_IS_ENABLED(BMP)
>  int splash_display(void)
>  {
>  	ulong addr;
>  	char *s;
>  	int x = 0, y = 0, ret;
> -
> +	if (!(CONFIG_IS_ENABLED(SPLASH_SCREEN)))
Remove extra braces as below :
if (!CONFIG_IS_ENABLED(SPLASH_SCREEN))
> +		return -ENOSYS;
>  	s = env_get("splashimage");
>  	if (!s)
>  		return -EINVAL;
> @@ -177,7 +175,10 @@ int splash_display(void)
>  
>  	splash_get_pos(&x, &y);
>  
> -	ret = bmp_display(addr, x, y);
> +	if (CONFIG_IS_ENABLED(BMP))
> +		ret = bmp_display(addr, x, y);
> +	else
> +		return -ENOSYS;
>  
>  	/* Skip banner output on video console if the logo is not at 0,0 */
>  	if (x || y)
> @@ -189,4 +190,3 @@ int splash_display(void)
>  end:
>  	return ret;
>  }
> -#endif
> diff --git a/include/splash.h b/include/splash.h
> index b6a100ffc3..9027f5d978 100644
> --- a/include/splash.h
> +++ b/include/splash.h
> @@ -61,20 +61,9 @@ static inline int splash_source_load(struct splash_location *locations,
>  

As Ifdefs are removed in my opinion blank lines are not needed after each
function declaration now,
>  int splash_screen_prepare(void);
>  

Remove this blank line
> -#if CONFIG_IS_ENABLED(SPLASH_SCREEN_ALIGN)
>  void splash_get_pos(int *x, int *y);
> -#else
> -static inline void splash_get_pos(int *x, int *y) { }
> -#endif
>  

Remove this blank line

With those changes,

Reviewed-by: Devarsh Thakkar <devarsht at ti.com>

Regards
Devarsh
> -#if CONFIG_IS_ENABLED(SPLASH_SCREEN) && CONFIG_IS_ENABLED(BMP)
>  int splash_display(void);
> -#else
> -static inline int splash_display(void)
> -{
> -	return -ENOSYS;
> -}
> -#endif
>  
>  #define BMP_ALIGN_CENTER	0x7FFF
>