On Wed, Dec 10, 2008 at 1:29 PM, Myles Watson <span dir="ltr"><<a href="mailto:mylesgw@gmail.com" target="_blank">mylesgw@gmail.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

<div><br>
<br>
> -----Original Message-----<br>
> From: <a href="mailto:coreboot-bounces@coreboot.org" target="_blank">coreboot-bounces@coreboot.org</a> [mailto:<a href="mailto:coreboot-bounces@coreboot.org" target="_blank">coreboot-bounces@coreboot.org</a>]<br>
> On Behalf Of Carl-Daniel Hailfinger<br>
> Sent: Wednesday, December 10, 2008 11:18 AM<br>
> To: Corey Osgood<br>
> Cc: Segher Boessenkool; coreboot<br>
> Subject: Re: [coreboot] [PATCH][v3] Check that CAR and ROM areas<br>
> don'tcollide<br>
><br>
</div><div>> Hi Segher,<br>
><br>
> is the last test below with 0x100000000 (2^32) in the formula guaranteed<br>
> to work or may cpp truncate the results to 32 bit? I'd rather avoid<br>
> introducing a test that can never trigger.<br>
<br>
</div>...snip...<br>
<div><br>
> What you actually want is this test:<br>
> #if CONFIG_CARBASE + CONFIG_CARSIZE + (CONFIG_COREBOOT_ROMSIZE_KB * 1024)<br>
> > 0x100000000<br>
<br>
</div>To avoid that problem, maybe we should /1024 instead of *.<br>
#if CONFIG_CARBASE/1024 + CONFIG_CARSIZE/1024 + CONFIG_COREBOOT_ROMSIZE_KB ><br>
1<<22<br>
<br>
I realize that 1<<22 isn't pretty, but the rest doesn't seem too bad.<br>
<br>
Thanks,<br>
<font color="#888888">Myles<br>
<br>
</font></blockquote></div><br>I'm probably missing something, but I'm not seeing the off-by-one error in the original function. CONFIG_CARBASE and 0xffffffff are both addresses, you could say they're both off by one, but by comparing them the problem is negated. You could rearrange it to <br>
<br>0xffffffff - (CONFIG_CARBASE + CONFIG_CARSIZE) < CONFIG_COREBOOT_ROMSIZE_KB * 1024<br><br>which should, AFAIK, compare the remaining space with the ROM size. Right?<br><br>
Thanks,<br>
Corey<br>