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

[Tom Rini]

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.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20241203/1c2b1c41/attachment.sig>