[U-Boot] [PATCH 6/8] tools: mkimage: Making table_entry code global

[Prafulla Wadaskar]

> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de] 
> Sent: Saturday, August 08, 2009 4:32 AM
> To: Prafulla Wadaskar
> Cc: u-boot at lists.denx.de; Ashish Karkare; Prabhanjan Sarnaik
> Subject: Re: [U-Boot] [PATCH 6/8] tools: mkimage: Making 
> table_entry code global
> 
> 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.
Dear Wolfgang,
I have gone through all the feedback that you have provided for this entire patch series.
Thanks a lot...

Now I need to take care of issues raised by you on the top of presenting them,
with comments wherever necessary to make it better readable and understandable.

I tried to maintain the flow in the series for easy understanding, but I failed :-(
With these patches, new code looks differently compared to original code.
So I feel instead of providing incremental patches- I should provide
1. basic mkimge framework patch,
2. mkimage: add: uimage support patch,
3. mkimage: add: fit image support patch,
4. mkimage: add: Kirkwood boot image support patch.
I will do this in my next post.
I hope you will like it.
If you have any inputs in this regard or you think this is bad plan, pls kindly let me know.

Regards..
Prafulla . .

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