[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