[U-Boot] [PATCH] include/image.h: Use explicit function definitions instead of macro trickery

Signed-off-by: Petri Lehtinen petri.lehtinen@inoi.fi ---
I got frustrated while grepping through the sources, trying to find the definitions of these functions. Thinking back, the obvious place to look in the first place would of course have been include/image.h, but it didn't even cross my mind that there could be some macro trickery behind these functions.
Anyway, I think that explicit is better than implicit and that this kind of macro usage is ugly.
include/image.h | 154 +++++++++++++++++++++++++++++++++++++++---------------- 1 files changed, 110 insertions(+), 44 deletions(-)
diff --git a/include/image.h b/include/image.h index 4609200..e3c33ca 100644 --- a/include/image.h +++ b/include/image.h @@ -333,28 +333,60 @@ static inline uint32_t image_get_header_size (void) return (sizeof (image_header_t)); }
-#define image_get_hdr_l(f) \ - static inline uint32_t image_get_##f(image_header_t *hdr) \ - { \ - return uimage_to_cpu (hdr->ih_##f); \ - } -image_get_hdr_l (magic); -image_get_hdr_l (hcrc); -image_get_hdr_l (time); -image_get_hdr_l (size); -image_get_hdr_l (load); -image_get_hdr_l (ep); -image_get_hdr_l (dcrc); - -#define image_get_hdr_b(f) \ - static inline uint8_t image_get_##f(image_header_t *hdr) \ - { \ - return hdr->ih_##f; \ - } -image_get_hdr_b (os); -image_get_hdr_b (arch); -image_get_hdr_b (type); -image_get_hdr_b (comp); +static inline uint32_t image_get_magic (image_header_t *hdr) +{ + return uimage_to_cpu (hdr->ih_magic); +} + +static inline uint32_t image_get_hcrc (image_header_t *hdr) +{ + return uimage_to_cpu (hdr->ih_hcrc); +} + +static inline uint32_t image_get_time (image_header_t *hdr) +{ + return uimage_to_cpu (hdr->ih_time); +} + +static inline uint32_t image_get_size (image_header_t *hdr) +{ + return uimage_to_cpu (hdr->ih_size); +} + +static inline uint32_t image_get_load (image_header_t *hdr) +{ + return uimage_to_cpu (hdr->ih_load); +} + +static inline uint32_t image_get_ep (image_header_t *hdr) +{ + return uimage_to_cpu (hdr->ih_ep); +} + +static inline uint32_t image_get_dcrc (image_header_t *hdr) +{ + return uimage_to_cpu (hdr->ih_dcrc); +} + +static inline uint8_t image_get_os (image_header_t *hdr) +{ + return hdr->ih_os; +} + +static inline uint8_t image_get_arch (image_header_t *hdr) +{ + return hdr->ih_arch; +} + +static inline uint8_t image_get_type (image_header_t *hdr) +{ + return hdr->ih_type; +} + +static inline uint8_t image_get_comp (image_header_t *hdr) +{ + return hdr->ih_comp; +}
static inline char *image_get_name (image_header_t *hdr) { @@ -386,33 +418,67 @@ static inline uint32_t image_get_image_size (image_header_t *hdr) { return (image_get_size (hdr) + image_get_header_size ()); } + static inline ulong image_get_image_end (image_header_t *hdr) { return ((ulong)hdr + image_get_image_size (hdr)); }
-#define image_set_hdr_l(f) \ - static inline void image_set_##f(image_header_t *hdr, uint32_t val) \ - { \ - hdr->ih_##f = cpu_to_uimage (val); \ - } -image_set_hdr_l (magic); -image_set_hdr_l (hcrc); -image_set_hdr_l (time); -image_set_hdr_l (size); -image_set_hdr_l (load); -image_set_hdr_l (ep); -image_set_hdr_l (dcrc); - -#define image_set_hdr_b(f) \ - static inline void image_set_##f(image_header_t *hdr, uint8_t val) \ - { \ - hdr->ih_##f = val; \ - } -image_set_hdr_b (os); -image_set_hdr_b (arch); -image_set_hdr_b (type); -image_set_hdr_b (comp); + +static inline void image_set_magic (image_header_t *hdr, uint32_t val) +{ + hdr->ih_magic = cpu_to_uimage (val); +} + +static inline void image_set_hcrc (image_header_t *hdr, uint32_t val) +{ + hdr->ih_hcrc = cpu_to_uimage (val); +} + +static inline void image_set_time (image_header_t *hdr, uint32_t val) +{ + hdr->ih_time = cpu_to_uimage (val); +} + +static inline void image_set_size (image_header_t *hdr, uint32_t val) +{ + hdr->ih_size = cpu_to_uimage (val); +} + +static inline void image_set_load (image_header_t *hdr, uint32_t val) +{ + hdr->ih_load = cpu_to_uimage (val); +} + +static inline void image_set_ep (image_header_t *hdr, uint32_t val) +{ + hdr->ih_ep = cpu_to_uimage (val); +} + +static inline void image_set_dcrc (image_header_t *hdr, uint32_t val) +{ + hdr->ih_dcrc = cpu_to_uimage (val); +} + +static inline void image_set_os (image_header_t *hdr, uint8_t val) +{ + hdr->ih_os = val; +} + +static inline void image_set_arch (image_header_t *hdr, uint8_t val) +{ + hdr->ih_arch = val; +} + +static inline void image_set_type (image_header_t *hdr, uint8_t val) +{ + hdr->ih_type = val; +} + +static inline void image_set_comp (image_header_t *hdr, uint8_t val) +{ + hdr->ih_comp = val; +}
static inline void image_set_name (image_header_t *hdr, const char *name) {

Dear Petri Lehtinen,
In message 1233125286-29550-1-git-send-email-petri.lehtinen@inoi.fi you wrote:
Signed-off-by: Petri Lehtinen petri.lehtinen@inoi.fi
I got frustrated while grepping through the sources, trying to find the definitions of these functions. Thinking back, the obvious place to look in the first place would of course have been include/image.h, but it didn't even cross my mind that there could be some macro trickery behind these functions.
Anyway, I think that explicit is better than implicit and that this kind of macro usage is ugly.
I am not really sure, but I tend to leave the code as it was.
-#define image_get_hdr_l(f) \
- static inline uint32_t image_get_##f(image_header_t *hdr) \
- { \
return uimage_to_cpu (hdr->ih_##f); \
- }
-image_get_hdr_l (magic); -image_get_hdr_l (hcrc); -image_get_hdr_l (time); -image_get_hdr_l (size); -image_get_hdr_l (load); -image_get_hdr_l (ep); -image_get_hdr_l (dcrc);
The macro here has the advantages of being compact, and that you need to change one location only to get a consistent set of function declarations.
Best regards,
Wolfgang Denk

On Wednesday 28 January 2009 16:20:13 Wolfgang Denk wrote:
Petri Lehtinen wrote:
I got frustrated while grepping through the sources, trying to find the definitions of these functions. Thinking back, the obvious place to look in the first place would of course have been include/image.h, but it didn't even cross my mind that there could be some macro trickery behind these functions.
Anyway, I think that explicit is better than implicit and that this kind of macro usage is ugly.
I am not really sure, but I tend to leave the code as it was.
-#define image_get_hdr_l(f) \
- static inline uint32_t image_get_##f(image_header_t *hdr) \
- { \
return uimage_to_cpu (hdr->ih_##f); \
- }
-image_get_hdr_l (magic); -image_get_hdr_l (hcrc); -image_get_hdr_l (time); -image_get_hdr_l (size); -image_get_hdr_l (load); -image_get_hdr_l (ep); -image_get_hdr_l (dcrc);
The macro here has the advantages of being compact, and that you need to change one location only to get a consistent set of function declarations.
agree completely.
Petri: if you're worried about grepping, then perhaps you'd post a patch like so: -image_get_hdr_l (dcrc); +image_get_hdr_l (dcrc); /* image_get_dcrc() */ -mike

Dear Mike Frysinger,
In message 200901281647.54413.vapier@gentoo.org you wrote:
Petri: if you're worried about grepping, then perhaps you'd post a patch like so: -image_get_hdr_l (dcrc); +image_get_hdr_l (dcrc); /* image_get_dcrc() */
Excellent idea!
Best regards,
Wolfgang Denk

Because the functions have been defined using macros, grepping for their definitions is not possible. This patch adds the real function names in comments.
Signed-off-by: Petri Lehtinen petri.lehtinen@inoi.fi --- include/image.h | 44 ++++++++++++++++++++++---------------------- 1 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/include/image.h b/include/image.h index 4609200..74a1240 100644 --- a/include/image.h +++ b/include/image.h @@ -338,23 +338,23 @@ static inline uint32_t image_get_header_size (void) { \ return uimage_to_cpu (hdr->ih_##f); \ } -image_get_hdr_l (magic); -image_get_hdr_l (hcrc); -image_get_hdr_l (time); -image_get_hdr_l (size); -image_get_hdr_l (load); -image_get_hdr_l (ep); -image_get_hdr_l (dcrc); +image_get_hdr_l (magic); /* image_get_magic */ +image_get_hdr_l (hcrc); /* image_get_hcrc */ +image_get_hdr_l (time); /* image_get_time */ +image_get_hdr_l (size); /* image_get_size */ +image_get_hdr_l (load); /* image_get_load */ +image_get_hdr_l (ep); /* image_get_ep */ +image_get_hdr_l (dcrc); /* image_get_dcrc */
#define image_get_hdr_b(f) \ static inline uint8_t image_get_##f(image_header_t *hdr) \ { \ return hdr->ih_##f; \ } -image_get_hdr_b (os); -image_get_hdr_b (arch); -image_get_hdr_b (type); -image_get_hdr_b (comp); +image_get_hdr_b (os); /* image_get_os */ +image_get_hdr_b (arch); /* image_get_arch */ +image_get_hdr_b (type); /* image_get_type */ +image_get_hdr_b (comp); /* image_get_comp */
static inline char *image_get_name (image_header_t *hdr) { @@ -396,23 +396,23 @@ static inline ulong image_get_image_end (image_header_t *hdr) { \ hdr->ih_##f = cpu_to_uimage (val); \ } -image_set_hdr_l (magic); -image_set_hdr_l (hcrc); -image_set_hdr_l (time); -image_set_hdr_l (size); -image_set_hdr_l (load); -image_set_hdr_l (ep); -image_set_hdr_l (dcrc); +image_set_hdr_l (magic); /* image_set_magic */ +image_set_hdr_l (hcrc); /* image_set_hcrc */ +image_set_hdr_l (time); /* image_set_time */ +image_set_hdr_l (size); /* image_set_size */ +image_set_hdr_l (load); /* image_set_load */ +image_set_hdr_l (ep); /* image_set_ep */ +image_set_hdr_l (dcrc); /* image_set_dcrc */
#define image_set_hdr_b(f) \ static inline void image_set_##f(image_header_t *hdr, uint8_t val) \ { \ hdr->ih_##f = val; \ } -image_set_hdr_b (os); -image_set_hdr_b (arch); -image_set_hdr_b (type); -image_set_hdr_b (comp); +image_set_hdr_b (os); /* image_set_os */ +image_set_hdr_b (arch); /* image_set_arch */ +image_set_hdr_b (type); /* image_set_type */ +image_set_hdr_b (comp); /* image_set_comp */
static inline void image_set_name (image_header_t *hdr, const char *name) {

On Thursday 29 January 2009 03:35:40 Petri Lehtinen wrote:
Because the functions have been defined using macros, grepping for their definitions is not possible. This patch adds the real function names in comments.
Signed-off-by: Petri Lehtinen petri.lehtinen@inoi.fi
Acked-by: Mike Frysinger vapier@gentoo.org -mike

Dear Petri Lehtinen,
In message 1233218140-11440-1-git-send-email-petri.lehtinen@inoi.fi you wrote:
Because the functions have been defined using macros, grepping for their definitions is not possible. This patch adds the real function names in comments.
Signed-off-by: Petri Lehtinen petri.lehtinen@inoi.fi
include/image.h | 44 ++++++++++++++++++++++---------------------- 1 files changed, 22 insertions(+), 22 deletions(-)
Applied, thanks.
Best regards,
Wolfgang Denk
participants (3)
-
Mike Frysinger
-
Petri Lehtinen
-
Wolfgang Denk