On Tue, 24 May 2011 09:01:13 +0200 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
Am 24.05.2011 02:21 schrieb Stefan Tauner:
after a short discussion with carldani i have removed the assert stuff and do some inferior array checking in selfcheck.
we only have _extern_ references to the tables to sizeof does not work. we could add a "getter" for the sizes of the array or bring the tables into view to improve this... but i dont think it's worth it.
IMHO assert should be avoided because it is the equivalent of crashing (no cleanup, no graceful error handling). Assert is OK if memory corruption or a similar problem has been detected and you just want to abort as quickly as possible without any cleanup to inimize further damage.
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.
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?
The code below is internal-only. Please make sure it still compiles with # make CONFIG_INTERNAL=no That usually means wrapping in #if CONFIG_INTERNAL == 1
did not think about that sorry, done.
Not your fault, but I think the \n at the beginning here is really ugly. It should live in the caller. […]
i have removed the \n at the start of all the print_supported_* functions and rearranged line breaks in print_supported in general a bit.
- 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.
new patch attached.