On 7/4/07, eran liberty eran.liberty@gmail.com wrote:
On 7/3/07, Grant Likely grant.likely@secretlab.ca wrote:
I might be missing something here; wouldn't it be better to extend the loadmk command? It looks to me that loadmk currently only handles uncompressed images, but that it could be fixed to do compressed ones too. Correct me if I'm wrong.
You are not wrong. Still I would like to argue 4 points
- To start with, when i wrote this functionality, loadmk was not
patched in yet... (not a very strong point :) )
- If we take a closer look at loadmk, its 10 lines long and this same
functionality is within my loadi under the "IH_COMP_NONE" case. (some what stornger)
- with the addition of the decompression functionality, loadmk will
be too long to be inlined in the the switch - case as it is right now. It would be exported to a function loadmk_func() and it will be called from the switch case. That is what loadi does. So you can either trash the original loadmk or rename my function to loadmk function and call it from loadmk... the out come is the same, only syntactical change. (Even stronger)
- I prefer the name loadi over loadmk. (The best argue in the world :) )
Heh; I also hate it when things change underneath me. :-) Regardless, you should consolidate. The name doesn't matter much, and loadmk was in first; so you should stick with loadmk. It's probably fine if you swap in your implementation, but make sure you cc the author of loadmk when you submit the patch.
+extern int gunzip (void *dst, int dstlen, unsigned char *src,
unsigned long *lenp);Very bad; please don't do this (even if it is done elsewhere). Please fix/create the appropriate gzip header file instead.
Agree! the gunzip function is not exported (defined in the header file) I wanted not to change any files I had no direct need to. should I add gunzip declaration to a header file?
Add the gunzip decl to the header please. Submit as a separate patch. For extra credit; include a patch to remove all the spurious extern gunzip() decls scattered about the various .c files. :-)
break;case IH_COMP_GZIP:if (gunzip((char *)CFG_LOAD_ADDR, unc_len, (uchar *) data, &len)!= 0) {nit: a little ugly; can you split the gunzip call and if() into separate lines for easy readability?
not ugly in my book, but can do. :)
Heh; only ugly because the if() is longer than 80chars and spills over 3 lines. :-)
Cheers, g.