[U-Boot] [PATCH 6/8] tools: mkimage: Making table_entry code global
Wolfgang Denk
wd at denx.de
Sat Aug 8 01:02:12 CEST 2009
Dear Prafulla Wadaskar,
In message <1248804270-13715-6-git-send-email-prafulla at marvell.com> you wrote:
>
> diff --git a/include/image.h b/include/image.h
> index 88a13ab..f119cee 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -168,6 +168,15 @@
> #define IH_MAGIC 0x27051956 /* Image Magic Number */
> #define IH_NMLEN 32 /* Image Name Length */
>
> +typedef struct table_entry {
> + int id; /* as defined in image.h */
> + char *sname; /* short (input) name */
> + char *lname; /* long (output) name */
> +} table_entry_t;
Now read this code again, with the distance of a couple of days, and
tell me what you think.
"as defined in image.h" - hey, this _is_ image.h !
So, "struct table_entry" - what sort of table is this?
"short (input) name", "long (output) name" - what the heck is this
about?
In the old version, the declaration of the struct was followed by the
declarations of the actual tables, so we could _see_ what was meant.
You rip the code out of context, which makes it unreadable and
ununderstandable.
This is a really bad idea, it seems.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"He only drinks when he gets depressed." "Why does he get depressed?"
"Sometimes it's because he hasn't had a drink."
- Terry Pratchett, _Men at Arms_
More information about the U-Boot
mailing list