Hi, * wu zhangjin <wuzhangjin@xxxxxxxxx> [2010-06-01 20:26:23+0800]: > > On Tue, Jun 1, 2010 at 6:56 PM, Alexander Clouter <alex@xxxxxxxxxxxxx> wrote: > > > > * Wu Zhangjin <wuzhangjin@xxxxxxxxx> [2010-06-01 18:29:03+0800]: > >> > >> We have calculated VMLINUZ_LOAD_ADDRESS in shell, which is awful. This patch > >> rewrites it in C. > >> > > I really feel that going down the C route is even worse....what's more > > this implementation is broken as it always returns with zero, even when > > sscanf() fails....and 'return -1' is just plain wrong too (look at > > sysexits.h for wisdom[1]). > > ooh, Sorry, Just found I have forgotten one "return", and for > portability, will use exit(EXIT_SUCCESS) and exit(EXIT_FAILURE) > instead later. > I *think* 'return EXIT_SUCCESS' is just as good if not better, as you have 'int main()' as your entry point...or whatever it is called. > > What is so 'awful' about the shell code version? > > From my point of view, it looks not good at least, and also not good > for maintaining. > Well, I guess I as you are the maintaining, what ever you says goes...more importantly whatever you are more comfortable with also. > > The shell lump is shorter in implementation size and I am personally not > > convinced any reasons hinting towards 'clarity' even apply as the shell > > code is well documented plus it is trivial to step through on any POSIX > > shell implementation; which cannot be said for the C code. > > I like shell too, herein, it is really shorter but is also hard to > understand, and of course, we need to ensure "unsigned long long" is > at least 64bit wide, I have tested it on my thinkpad SL400 laptop(X86) > and my Yeeloong netbook(MIPS), both of them works well. > > > I am also not too confident 'unsigned long long' is a great > > idea...maybe 'u64' or 'uint64_t' if you are relying on C99[1]? > > good idea, which c header file defines u64 and uint64_t? > Wackipedia claims[1] that for C99 you want stdint.h, a quick grep of my /usr/include directory agrees too. It seems that you want 'uint64_t' too and not 'u64' which is Linux only...or something. Cheers [1] http://en.wikipedia.org/wiki/Stdint.h -- Alexander Clouter .sigmonster says: Gibble, Gobble, we ACCEPT YOU ...
