Re: [ECOS] Re: Check for illegal address range in io/flash

[Gary Thomas <gary@mlbassoc.com>]

Øyvind Harboe wrote:
> On Dec 11, 2007 2:51 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> On Tue, Dec 11, 2007 at 11:09:40AM +0100, ?yvind Harboe wrote:
>>> On Dec 11, 2007 11:03 AM, Andrew Lunn <andrew@lunn.ch> wrote:
>>>>> I mainly want the busted check in flash_erase() to be removed.
>>>>> ASSERT's are probably fine.
>> What is actually wrong with this check?
> 
> I have no idea what this check is supposed to do.
> 
> If a valid range is passed in, it is a no-operation, otherwise it does
> something mysterious
> that looks just wrong to me.
> 
> I'm afraid all I can conclude is that it should be deleted as plain
> wrong/unused. The
> fact that it is harmless for valid ranges has allowed it to survive to date,
> that's the antrophic principle for you I guess.
> 
> Someone else would have to speak up for it.
> 
> It would be a shame if plain wrong harmless code that nobody understands
> could never be deleted from eCos :-)
> 
>> I don't want to remove this until i fully understand what is causing
>> your problem and how this code is wrong. Please could you explain what
>> you are seeing.
> 
> The problem is that I relied on flash_erase() to return an error upon illegal
> address range. When I tested with an illegal address range,
> flash_erase() did not
> return an error. Upon inspecting the code, I saw something that looked
> like it is *trying* to check the address, but I have found no explanation
> for that code, so I see a couple of alternatives:
> 
> - if nobody understands this code, delete it after a week or two to
> wait for someone to
> speak up and see what happens. Probably nothing.
> - keep this code forever as it is harmless(with valid addresses)
> nobody will be hurt by it.
> - add address check to flash_erase() & program and return error message

As I read the code, the "check" you are talking about is only to make
sure that the address [range] which gets printed makes sense.  There is
similar code within the loop to handle the edge case where the FLASH
region being erased buts up against the top of possible address space.

I don't see any error checking present at all.


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss