[PATCH v4 03/25] efi_loader: Drop extra brackets in efi_mem_carve_out()

[Simon Glass]

Hi Tom,

On Tue, 3 Dec 2024 at 07:04, Tom Rini <trini at konsulko.com> wrote:
>
> On Tue, Dec 03, 2024 at 06:46:11AM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Mon, 2 Dec 2024 at 17:37, Tom Rini <trini at konsulko.com> wrote:
> > >
> > > On Mon, Dec 02, 2024 at 05:24:35PM -0700, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Mon, 2 Dec 2024 at 13:16, Tom Rini <trini at konsulko.com> wrote:
> > > > >
> > > > > On Sun, Dec 01, 2024 at 08:24:22AM -0700, Simon Glass wrote:
> > > > > > Simplify a few expressions in this function.
> > > > > >
> > > > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > > > > ---
> > > > > >
> > > > > > (no changes since v1)
> > > > > >
> > > > > >  lib/efi_loader/efi_memory.c | 4 ++--
> > > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> > > > > > index f1154f73e05..3b1c7528e92 100644
> > > > > > --- a/lib/efi_loader/efi_memory.c
> > > > > > +++ b/lib/efi_loader/efi_memory.c
> > > > > > @@ -206,11 +206,11 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map,
> > > > > >                            (carve_desc->num_pages << EFI_PAGE_SHIFT);
> > > > > >
> > > > > >       /* check whether we're overlapping */
> > > > > > -     if ((carve_end <= map_start) || (carve_start >= map_end))
> > > > > > +     if (carve_end <= map_start || carve_start >= map_end)
> > > > > >               return EFI_CARVE_NO_OVERLAP;
> > > > > >
> > > > > >       /* We're overlapping with non-RAM, warn the caller if desired */
> > > > > > -     if (overlap_conventional && (map_desc->type != EFI_CONVENTIONAL_MEMORY))
> > > > > > +     if (overlap_conventional && map_desc->type != EFI_CONVENTIONAL_MEMORY)
> > > > > >               return EFI_CARVE_OVERLAPS_NONRAM;
> > > > > >
> > > > > >       /* Sanitize carve_start and carve_end to lie within our bounds */
> > > > >
> > > > > As I believe was mentioned in a previous iteration, please drop this as
> > > > > they aren't excessive generates a compiler warning, merely for
> > > > > clarification and should be kept.
> > > >
> > > > I did this patch because checkpatch complained and I am changing these lines.
> > >
> > > And checkpatch is not the authority, it's guidelines.
> >
> > Agreed.
> >
> > > I believe the
> > > review comments are "no, these should stay". Please drop this patch.
> >
> > Just so I can figure out what to do here, are you saying:
> > - merge this patch in with the one that produces a checkpatch warning
> > (i.e. remove brackets so resolve warning), or
> > - drop this patch and ignore the checkpatch warning in the result
>
> Yes, please just drop this patch, I only look at ERROR from checkpatch
> when merging things and then it's a matter of what the error is.
>
> > I don't really mind about this, obviously. But as I suspect this
> > series is not going to be applied to your tree anyway, I'll await
> > events.
>
> Yes, if you're not going to be addressing most of the comments you get
> this is going to linger in, well, whatever it is you plan to be doing
> exactly.

I am keen to address comments, so long as they are about the code,
rather than 'we don't want this patch'.

Regards,
SImon