diff mbox series

[v4,6/8] image: Do not #if guard board_fit_image_post_process() prototype

Message ID 20210120164656.1396639-7-mr.nuke.me@gmail.com
State Accepted
Delegated to: Tom Rini
Headers show
Series spl: fit: Play nicely with OP-TEE and Linux | expand

Commit Message

Alexandru Gagniuc Jan. 20, 2021, 4:46 p.m. UTC
There's no point in guarding function prototypes with #ifdefs. If a
function is not defined, the linker will notice. Having the prototype
does not affect code size.

What the #if guard takes away is the ability to use IS_ENABLED:

	if (CONFIG_IS ENABLED(FIT_IMAGE_POST_PROCESS))
		board_fit_image_post_process(...)

When the prototype is guarded, the above form cannot be used. This
leads to the proliferation of #ifdefs, and unreadable code. The
opportunity cost of the #if guard outweighs any benefits. Remove it.

Since the original version of this patch, an empty definition was
added by commit f14e6eec6c7f ("image: cleanup pre-processor usage").
The empty definition can cause silent failures, when an implementation
of board_fit_image_post_process() is expected because the linker will
not catch the missing function. Thus this patch removes this empty
inline declaration.

Fixes: f14e6eec6c7f ("image: cleanup pre-processor usage")
Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 include/image.h | 7 -------
 1 file changed, 7 deletions(-)

Comments

Tom Rini Feb. 19, 2021, 4:55 p.m. UTC | #1
On Wed, Jan 20, 2021 at 10:46:54AM -0600, Alexandru Gagniuc wrote:

> There's no point in guarding function prototypes with #ifdefs. If a
> function is not defined, the linker will notice. Having the prototype
> does not affect code size.
> 
> What the #if guard takes away is the ability to use IS_ENABLED:
> 
> 	if (CONFIG_IS ENABLED(FIT_IMAGE_POST_PROCESS))
> 		board_fit_image_post_process(...)
> 
> When the prototype is guarded, the above form cannot be used. This
> leads to the proliferation of #ifdefs, and unreadable code. The
> opportunity cost of the #if guard outweighs any benefits. Remove it.
> 
> Since the original version of this patch, an empty definition was
> added by commit f14e6eec6c7f ("image: cleanup pre-processor usage").
> The empty definition can cause silent failures, when an implementation
> of board_fit_image_post_process() is expected because the linker will
> not catch the missing function. Thus this patch removes this empty
> inline declaration.
> 
> Fixes: f14e6eec6c7f ("image: cleanup pre-processor usage")
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!
diff mbox series

Patch

diff --git a/include/image.h b/include/image.h
index 856bc3e1b2..5cd143856a 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1537,8 +1537,6 @@  bool android_image_print_dtb_contents(ulong hdr_addr);
  */
 int board_fit_config_name_match(const char *name);
 
-#if defined(CONFIG_SPL_FIT_IMAGE_POST_PROCESS) || \
-	defined(CONFIG_FIT_IMAGE_POST_PROCESS)
 /**
  * board_fit_image_post_process() - Do any post-process on FIT binary data
  *
@@ -1553,11 +1551,6 @@  int board_fit_config_name_match(const char *name);
  * @return no return value (failure should be handled internally)
  */
 void board_fit_image_post_process(void **p_image, size_t *p_size);
-#else
-static inline void board_fit_image_post_process(void **p_image, size_t *p_size)
-{
-}
-#endif /* CONFIG_SPL_FIT_IMAGE_POST_PROCESS */
 
 #define FDT_ERROR	((ulong)(-1))