[flashrom] [PATCH] LLVM/clang code review

[Carl-Daniel Hailfinger]

On 16.09.2009 10:25, Stefan Reinauer wrote:
> Carl-Daniel Hailfinger wrote:
>   
>> On 15.09.2009 19:49, Stefan Reinauer wrote:
>>   
>>     
>>> this patch fixes all 27 flashrom source code issues reported by
>>> LLVM/clang's scan-build (r79326, new build on the way).
>>>
>>> Signed-off-by: Stefan Reinauer <stepan at coresystems.de>  
>>>     
>>>       
>> The following parts of the patch are
>> Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
>>
>> sst28sf040.c (definitely)
>> sharplhf00l04.c (we shouldn't enter ID mode at all)
>> stm50flw0x0x.c (we shouldn't enter ID mode at all)
>> serprog.c (serprog_die should be killed, but for now...)  
>>     
> Ok, r722
>   

Thanks!


>> I can do the ID mode removal in a followup patch.
>>
>>
>> The serprog.c change is correct, but we need a design review of serprog
>> anyway and I hope to kill serprog_die() completely in that review.
>>   
>>     
> Don't let my fixes keep you from doing further work on the code.
>   

I was thinking about postponing the immediate fix for a longterm
cleanup, but I think that will need a bit more time and the postponement
wouldn't have any purpose except turning up on scan-build and reminding
us of the larger cleanup... so in the end I'm happy you applied this.


>> ft2232_spi.c is a bit special because I don't have any idea what
>> port_val is used for. It looks like a dead variable, but maybe it was
>> intended to track the status of other lines besides CS# and in that case
>> we should keep the variable and fill it with the correct value.
>>   
>>     
> It is a dead variable. At least in this instance of code. If this is not
> the way it's supposed to be, we still want to get rid of it so it is
> easier to spot that part in the future.
>   

True, see my reaction to Paul's mail. No objection anymore.


>> In 82802ab.c I miss some of the commented out commands. Some of them
>> should be fixed and reenabled (especially the chip protection status).
>>   
>>     
> I didn't comment out or remove anny commands, just debug output.
>   

My apologies. The wording was a bit unclear. You removed some commands
(write protect status printing) which were already commented out and
thus inactive. I believe they shouldn't have been commented out in the
first place. I'll send a patch to restore them into full effect on top
of r723.

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/