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

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@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(-)

On Wed, 19 Apr 2023 at 18:11, Nikhil M Jain <n-jain1@ti.com> 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@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(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

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@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@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
>