Hi Tom, On Tue, May 14, 2013 at 3:28 PM, Tom Rini <tr...@ti.com> wrote: > On Tue, May 14, 2013 at 03:22:04PM -0700, Vadim Bendebury wrote: >> On Tue, May 14, 2013 at 3:15 PM, Tom Rini <tr...@ti.com> wrote: >> > On Tue, May 14, 2013 at 02:27:32PM -0700, Simon Glass wrote: >> >> Hi Tom, >> >> >> >> On Tue, May 14, 2013 at 2:21 PM, Tom Rini <tr...@ti.com> wrote: >> >> > On Mon, May 13, 2013 at 03:12:07PM -0700, Simon Glass wrote: >> >> >> Hi Tom, >> >> >> >> >> >> On Thu, Mar 21, 2013 at 7:38 AM, Tom Rini <tr...@ti.com> wrote: >> >> >> > On Tue, Feb 26, 2013 at 08:10:53AM -0800, Simon Glass 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. >> >> >> >> >> >> >> >> The use of #ifdef in U-Boot has possibly got a little out of hand. >> >> >> >> In an >> >> >> >> attempt to turn the tide, this series includes a patch which >> >> >> >> provides a way >> >> >> >> to make CONFIG macros available to C code without using the >> >> >> >> preprocessor. >> >> >> >> This makes it possible to use standard C conditional features such >> >> >> >> as >> >> >> >> if/then instead of #ifdef. A README update exhorts compliance. >> >> >> > >> >> >> > OK, this is true. Looking over the series, a number of the patches >> >> >> > are >> >> >> > just general fixes / improvements that don't depend on the >> >> >> > autoconf_... >> >> >> > work. Lets split this out now and take them in now as they seem like >> >> >> > reviewable by inspection code. >> >> >> >> >> >> Well sorry I didn't make time to get this done last time. I can do >> >> >> this now or... >> >> >> >> >> >> > >> >> >> > For the approach itself, I'm not sure which is best here. In a lot >> >> >> > of >> >> >> > cases we're trading an #ifdef for adding a level of indent to already >> >> >> > pretty indented code and that feels like a wash, in terms of >> >> >> > readability >> >> >> > to me. We probably need to re-factor some of the code in question >> >> >> > there >> >> >> > too for readability, then see about using autoconf_... type things, >> >> >> > or >> >> >> > maybe something else. >> >> >> >> >> >> I think you are saying to do the rearrangement and clean-up first, >> >> >> then add autoconf afterwards. I can do that but really I am wondering >> >> >> what you think of the autoconf concept? The Kconfig stuff is related >> >> >> here too, but first I would like to decide on what to do with the >> >> >> #ifdefs. >> >> > >> >> > I think a lot of our #ifdefery is a problem of code that's in need of >> >> > some love and re-org and cleaning and updating. One of the old style >> >> > rules I still try and follow is that after a few levels of indent code >> >> > doesn't read well. Also big nested #ifdefs don't read well. So we're >> >> > trading one in for the other. But your series showed a lot of places >> >> > where we can re-factor things to improve readability. So lets go that >> >> > way. Then we can see if there's still things to improve on, and what >> >> > dead code we still have around. >> >> >> >> So are you saying that you are keen on the autoconf idea? >> > >> > I'm saying lets clean up the code and see if we still need something >> > like autoconf. It seems to provide the most benefit in terms of >> > readability in places that could read a lot better with some clean up >> > and refactoring before we add new tools to the pile. >> > >> >> Yet another great advantage of autoconf is that it ensures a >> consistent combination of the configuration options, with the status >> quo it is so easy to make a mistake and create a deficient >> configuration. > > There are things I like about the concept, but I really want to see the > problem areas in question made more readable as it will both help in > general, and if we do make this conversion later, make the conversion > easier as we'll hopefully kill off some of the nested and tricky ifdefs.
I've brought over the patches that I can that don't depend on autoconf: 6c0e6c9 (HEAD, ws/us-config5, us-config5) main: Add debug_bootkeys to avoid #ifdefs 9777b9f main: Add debug_parser() to avoid #ifdefs 2fc85b6 main: Correct header order 24e80be main: Fix typos and checkpatch warnings in command line reading 9e9e3b9 main: Use get/setenv_ulong() 1290cb7 main: Move boot_delay code into its own function cd8f13e main: Separate out the two abortboot() functions ca2451c net: Add prototype for update_tftp 4a2a802 at91: Correct CONFIG_AUTOBOOT_PROMPT definition for pm9263 I will run these through the builder and send an interim series without autoconf. We still have a lot of inline #ifdefs, and static functions and local variables must also be #ifdefed out if not used. Still, there is some improvement. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
