Hi Tom, On Mon, Oct 28, 2013 at 9:32 AM, Tom Rini <[email protected]> wrote: > On Mon, Oct 28, 2013 at 03:44:01PM +0100, Wolfgang Denk wrote: >> Dear Simon, >> >> In message <[email protected]> you wrote: >> > Many parts of the U-Boot code base are sprinkled with #ifdefs. This makes >> > different boards compile different versions of the source code, meaning >> > that we must build all boards to check for failures. It is easy to misspell >> > an #ifdef and there is not as much checking of this by the compiler. >> > Multiple >> > dependent #ifdefs are harder to do than with if..then..else. Variable >> > declarations must be #idefed as well as the code that uses them, often much >> > later in the file/function. #ifdef indents don't match code indents and >> > have their own separate indent feature. Overall, excessive use of #idef >> > hurts readability and makes the code harder to modify and refactor. For >> > people coming newly into the code base, #ifdefs can be a big barrier. >> >> While I agree in general with the goal and the implementation, there >> is an implementation detail I dislike - this is that the new code >> is harder to type and to read - I mean, something like >> CONFIG_SYS_HUSH_PARSER is already clumsy enough, but making this >> autoconf_sys_hush_parser() appears to be worse. Also, the connection >> to a CONFIG_* option is not easily visible. >> >> >> I also had feedback from Detlev (who is unfortunately on a business >> trip again so he didn't find time [yet] to comment himself); he >> commented that he really likes the idea, but does not like that we now >> have to access the well-known contants using a new name. >> >> >> Maybe we can find a shorter / easier to read way to do this - I think >> it would be really nice if we could see the well-known names. > > I guess this is what I get for not being like Linus sometimes[1]. I think > what this series highlights is that we have a lot of code in > common/main.c that needs either re-thinking, re-factoring or splitting > out into difference functions and files. And when we're turning > #ifdef CONFIG_FOO > ... whatever ... > #endif > into > if (autoconf_foo()) { > ... whatever ... > } > > The code starts looking worse in some cases since we're already 3 > indents in. The problem is we have lots of ifdef code, in some areas of > the code. This hides the problem, not fixes the problem.
While I agree with this, the question is more whether using the compiler instead of the preprocessor helps with reducing the number of code paths and the number of boards we must build to test things. In many cases the CONFIG options hide features that we don't want to enable. I did a series that improved main.c in various ways including splitting the code differently - there are a few more things that could be done, but it will still be a mountain of #ifdefs. I would quite like to split the command editing stuff into its own file - in fact main.c could be split into several files: - command line - command editing - parser But does this help? I wasn't entirely sure. > > Looking over the first parts of the series, weak functions for example > would help in a number of cases, especially if we split things out of > main.c and into other files too. I am not a huge fan of weak functions since it isn't clear what happens when they are called. But again if you have a specific example it would help me understand your intent here. Regards, Simon > > -- > Tom > > [1]: By which I mean being very "forceful" when saying I don't like an > idea. BTW I am not sure about this idea either - let's figure out exactly what it can help with and whether it is worth it. Perhaps main.c was not the best choice - but board files have loads of #ifdefs also. _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
