[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