[coreboot] [PATCH] CONFIG_LB_MEM_TOPK -> CONFIG_RAMTOP

Peter Stuge peter at stuge.se
Fri Oct 16 18:42:22 CEST 2009


Myles Watson wrote:
> > > There was only one place where I had to do (RAMBASE >> 10)!
> >
> > Maybe change that one to / 1024 instead?
> 
> Do you think it's clearer to use /1024?

Yes, very much so.


> I just meant it was surprising to multiply the value by 1024 and
> only have to put it back once.

I agree with that.


> > In a few places CONFIG_RAMTOP is used without (), but none of them
> > looked bad. Maybe also search for ",CONFIG_RAMTOP" and add a space
> > there.
> 
> Fixed.

Great!


ron minnich wrote:
> I prefer >>10 because it more clearly expresses what you want.

I strongly prefer to get kilobytes from bytes using 1024 rather than 10.

I don't think the bit operator is more clear - this isn't a bit
operation, it's arithmetic.

The particular arithmetic happens to have an efficient implementation
using a bit operator, but the compiler knows that, so that the
programmer doesn't have to care and can use arithmetic expression,
which is meant to be (I think it is) more clear.


> I've been burned from time to time by people using an arithmetic op
> where a bit op was intended. (+ instead of | for example)

That would be a bug, but it doesn't translate to "all use of
arithmetic operators should be changed to use bit operators".

Not in C. For assembly you could convince me, but not in C. :)


//Peter




More information about the coreboot mailing list