[flashrom] [PATCH] eliminate magic numbers indicating maximum column sizes in print_supported_chipsets and print_supported_boards_helper

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Thu May 26 01:34:28 CEST 2011


Am 25.05.2011 18:19 schrieb Stefan Tauner:
> On Tue, 24 May 2011 09:01:13 +0200 Carl-Daniel Hailfinger wrote:
>   
>> The tests for NULL pointers of chipset_enables etc. make a lot of sense, 
>> but the size checks only make sense if you know the array size from 
>> elsewhere and if you want to check that the array size matches the size 
>> an array walker (loop until ->somemember==NULL) would see. That means 
>> the selfcheck function would have to live in the same file as the array 
>> or you use another global variable which holds sizeof(array).
>>     
> or making the size accessible out of scope with global functions, yes.
> do we want that? i think it's ok for now.
>   

OK.


> you did not mention "flashchips". i would not think that there exists a
> minimal use case for having that one empty, but just to be sure:
> the application of "|| flashchips[0].vendor == NULL" is ok there?
>   

Yes.


>> Am 24.05.2011 02:21 schrieb Stefan Tauner:
>>> -	int status;
>>> +	int status; /* OK=0 and NT=1 are defines only. Beware! */   
>>>       
>> Do we want an enum instead?
>>     
> i like strong types, but i dont care too much in this case.
>   

If we use an enum here, we should also use an enum everywhere else for
tested/untested.


> new patch attached.
>   

Looks good, thanks for addressing the review comments. I assume you
tested that the output still looks OK and that it compiles.
Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>

Please go ahead and commit.

Regards,
Carl-Daniel

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





More information about the flashrom mailing list