[U-Boot] [PATCH v8 4/9] x86: slimbootloader: Add memory configuration

[Andy Shevchenko]

On Fri, Aug 2, 2019 at 1:19 PM Park, Aiden <aiden.park at intel.com> wrote:
> > -----Original Message-----
> > From: Andy Shevchenko [mailto:andy.shevchenko at gmail.com]
> > Sent: Friday, August 2, 2019 4:51 PM
> > To: Park, Aiden <aiden.park at intel.com>
> > Cc: Bin Meng <bmeng.cn at gmail.com>; U-Boot Mailing List <u-
> > boot at lists.denx.de>; Simon Glass <sjg at chromium.org>
> > Subject: Re: [PATCH v8 4/9] x86: slimbootloader: Add memory
> > configuration
> > On Fri, Aug 2, 2019 at 10:49 AM Andy Shevchenko
> > <andy.shevchenko at gmail.com> wrote:
> > > On Fri, Aug 2, 2019 at 10:03 AM Park, Aiden <aiden.park at intel.com>
> > wrote:

> > > > +#define for_each_memory_map_entry_reversed(iter, entries) \
> > > > +       for (iter = entries->count - 1; iter >= 0; iter--) \
> > > > +               if (entries->entry[iter].type == E820_RAM)
> > > > +
> > >
> > > It seems you missed my answer to Bin.
> > > This is simple incorrect. Checkpatch sometimes is wrong.
> >
> > Let me elaborate why.
> >
> > The idea of having
> >
> >  if (foo) {} else
> >
> > pattern is to avoid weirndess like
> >
> >   for_each_...() {
> >     ...
> >   } else {
> >     ...WTF!..
> >   }
> >
> Sorry for missing your comment. I understand '{} else' is better way.
> It looks checkpatch issue is bigger scope than this one.
> What about this in this series?
>
> #define for_each_if(condition) if (!(condition)) {} else
> #define for_each_memory_map_entry_reversed(iter, entries) \
>         for (iter = entries->count - 1; iter >= 0; iter--) \
>                for_each_if(entries->entry[iter].type == E820_RAM)
>
> tricky, but checkpatch does not report ERROR.

Fine for me. If Bin is okay with it, it will be good then.

-- 
With Best Regards,
Andy Shevchenko