Re: [PATCH] efi/fdt: More cleanups

[Ingo Molnar <mingo@kernel.org>]

* Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> On Fri, 30 Nov 2018 at 08:57, Ingo Molnar <mingo@kernel.org> wrote:
> >
> >
> > * Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >
> > > From: Julien Thierry <julien.thierry@arm.com>
> > >
> > > Closing bracket seems to end a for statement when it is actually ending
> > > the contained if. Add some brackets to have clear delimitation of each
> > > scope.
> > >
> > > No functional change/fix, just fix the indentation.
> > >
> > > Signed-off-by: Julien Thierry <julien.thierry@arm.com>
> > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > ---
> > >  drivers/firmware/efi/libstub/fdt.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
> > > index 0c0d2312f4a8..a3614f9b5f75 100644
> > > --- a/drivers/firmware/efi/libstub/fdt.c
> > > +++ b/drivers/firmware/efi/libstub/fdt.c
> > > @@ -376,7 +376,7 @@ void *get_fdt(efi_system_table_t *sys_table, unsigned long *fdt_size)
> > >       tables = (efi_config_table_t *) sys_table->tables;
> > >       fdt = NULL;
> > >
> > > -     for (i = 0; i < sys_table->nr_tables; i++)
> > > +     for (i = 0; i < sys_table->nr_tables; i++) {
> > >               if (efi_guidcmp(tables[i].guid, fdt_guid) == 0) {
> > >                       fdt = (void *) tables[i].table;
> > >                       if (fdt_check_header(fdt) != 0) {
> > > @@ -385,7 +385,8 @@ void *get_fdt(efi_system_table_t *sys_table, unsigned long *fdt_size)
> > >                       }
> > >                       *fdt_size = fdt_totalsize(fdt);
> > >                       break;
> > > -      }
> > > +             }
> > > +     }
> >
> > So if we are doing trivial cleanups, how about the patch below on top? It
> > cleans up this file for good. Only minimally build tested.
> >
> > Thanks,
> >
> >         Ingo
> >
> > ======================>
> > Subject: efi/fdt: More cleanups
> > From: Ingo Molnar <mingo@kernel.org>
> >
> > Apply a number of cleanups:
> >
> >  - Introduce fdt_setprop_*var() helper macros to simplify and shorten repetitive
> >    sequences - this also makes it less likely that the wrong variable size is
> >    passed in. This change makes a lot of the property-setting calls single-line
> >    and easier to read.
> >
> 
> This change looks fine to me, but scripts/dtc/libfdt/libfdt.h is part
> of an external library, so we'd either need to contribute it there or
> define the macro in a local header.

Whichever variant you prefer. If the external library has a different 
license that requires a separate permission then for that too feel free 
to add:

  Signed-off-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo