[U-Boot] [PATCH v3] moveconfig: add a second pass for empty #if/#endif blocks

[Masahiro Yamada]

On Wed, Jan 30, 2019 at 5:55 PM Masahiro Yamada
<yamada.masahiro at socionext.com> wrote:
>
> On Wed, Jan 30, 2019 at 4:23 PM Chris Packham <judge.packham at gmail.com> wrote:
> >
> > Moveconfig already attempts to remove empty #if/#endif blocks when there
> > is a matching CONFIG_ being moved. Add a second pass which covers files
> > without a match.
> >
> > Signed-off-by: Chris Packham <judge.packham at gmail.com>
> > ---
> > This was previously submitted as
> > http://patchwork.ozlabs.org/patch/924901/ there still seems to be cases
> > of #if/#endif left over from Kconfig migrations so perhaps this is still
> > needed/wanted.
> >
> > I've plumbed this in as a second pass because ultimately we may want to
> > make this a separate option. Also I couldn't figure out how to implement
> > this without using re.M so I couldn't make it work in with the line by
> > line parsing of cleanup_one_header().
>
>
> This seems useful to find leftover code
> regardless of the CONFIG options you are moving.
>
> One drawback is, it may fold unrelated cleanups.
>
> Maybe, it would be useful to add a separate option to turn on this feature,
> or make it into a separate tool.
>
>
>


I tested this patch.
It detected one empty block.


diff --git a/include/configs/omap3_cairo.h b/include/configs/omap3_cairo.h
index 04bce2f..ac1a6cb 100644
--- a/include/configs/omap3_cairo.h
+++ b/include/configs/omap3_cairo.h
@@ -197,8 +197,6 @@
  * are needed and peripheral clocks for UART2 must be enabled in
  * function per_clocks_enable().
  */
-#ifdef CONFIG_SPL_BUILD
-#endif

 /* Provide the MACH_TYPE value the vendor kernel requires */
 #define CONFIG_MACH_TYPE       3063







This seems a leftover of 9baa2bce.


$ git show 9baa2bce -- include/configs/omap3_cairo.h
commit 9baa2bce28901321d6f62399b5ebeb3fcb8e8a57
Author: Adam Ford <aford173 at gmail.com>
Date:   Tue Aug 7 07:08:32 2018 -0500

    Removed unused references to CONFIG_SERIALx

    After creating CONS_INDEX and migrating a bunch of boards to it,
    there are a bunch of defined references to CONFIG_SERIALx which
    are not referenced in any C code or #ifdef, so they can now be
    removed

    Signed-off-by: Adam Ford <aford173 at gmail.com>

diff --git a/include/configs/omap3_cairo.h b/include/configs/omap3_cairo.h
index 0133416..04bce2f 100644
--- a/include/configs/omap3_cairo.h
+++ b/include/configs/omap3_cairo.h
@@ -198,8 +198,6 @@
  * function per_clocks_enable().
  */
 #ifdef CONFIG_SPL_BUILD
-#undef CONFIG_SERIAL3
-#define CONFIG_SERIAL2
 #endif

 /* Provide the MACH_TYPE value the vendor kernel requires */





However, I doubt it used the moveconfig tool.


I re-ran the tool against the previous commit.


$ git checkout 9baa2bce2890^
$ tools/moveconfig.py -H SERIAL0 SERIAL1 SERIAL2 SERIAL3
Clean up headers? [y/n]: y
y
...
$ git diff -- include/configs/omap3_cairo.h
diff --git a/include/configs/omap3_cairo.h b/include/configs/omap3_cairo.h
index 0133416..ac1a6cb 100644
--- a/include/configs/omap3_cairo.h
+++ b/include/configs/omap3_cairo.h
@@ -197,10 +197,6 @@
  * are needed and peripheral clocks for UART2 must be enabled in
  * function per_clocks_enable().
  */
-#ifdef CONFIG_SPL_BUILD
-#undef CONFIG_SERIAL3
-#define CONFIG_SERIAL2
-#endif

 /* Provide the MACH_TYPE value the vendor kernel requires */
 #define CONFIG_MACH_TYPE       3063





cleanup_one_header() did a good job.

Is there a particular case that the second path is needed?



-- 
Best Regards
Masahiro Yamada