Re: [PATCH] MIPS: Clean up the calculation of VMLINUZ_LOAD_ADDRESS

[Alexander Clouter <alex@digriz.org.uk>]

Hi,

* wu zhangjin <wuzhangjin@gmail.com> [2010-06-01 20:26:23+0800]:
>
> On Tue, Jun 1, 2010 at 6:56 PM, Alexander Clouter <alex@digriz.org.uk> wrote:
> >
> > * Wu Zhangjin <wuzhangjin@gmail.com> [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 ...