| Message ID | 20241208105223.2821049-3-ilias.apalodimas@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Cleanup the LMB subsystem | expand |
Am 8. Dezember 2024 11:52:05 MEZ schrieb Ilias Apalodimas <ilias.apalodimas@linaro.org>: >There's no point subtracting -1 from the calculated addresses and then >check for a <= b. Just remove the -1 and check for a < b. I once thought that, too. But it makes a difference for end= U(L)LONG_MAX. Best regards Heinrich > >Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> >--- > lib/lmb.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > >diff --git a/lib/lmb.c b/lib/lmb.c >index a7ecbb58831f..c7bf5120696f 100644 >--- a/lib/lmb.c >+++ b/lib/lmb.c >@@ -36,10 +36,10 @@ DECLARE_GLOBAL_DATA_PTR; > static long lmb_addrs_overlap(phys_addr_t base1, phys_size_t size1, > phys_addr_t base2, phys_size_t size2) > { >- const phys_addr_t base1_end = base1 + size1 - 1; >- const phys_addr_t base2_end = base2 + size2 - 1; >+ const phys_addr_t base1_end = base1 + size1; >+ const phys_addr_t base2_end = base2 + size2; > >- return ((base1 <= base2_end) && (base2 <= base1_end)); >+ return ((base1 < base2_end) && (base2 < base1_end)); > } > > static long lmb_addrs_adjacent(phys_addr_t base1, phys_size_t size1,
Hi Heinrich On Sun, 8 Dec 2024 at 13:31, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > Am 8. Dezember 2024 11:52:05 MEZ schrieb Ilias Apalodimas <ilias.apalodimas@linaro.org>: > >There's no point subtracting -1 from the calculated addresses and then > >check for a <= b. Just remove the -1 and check for a < b. > > I once thought that, too. But it makes a difference for end= U(L)LONG_MAX. If those addresses overflow then we have to add more checks -- e.g if (base2_end < base2) { Do something wrong } In which case, I prefer modifying the function and add those checks Thoughts? /Ilias > > Best regards > > Heinrich > > > > >Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > >--- > > lib/lmb.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > >diff --git a/lib/lmb.c b/lib/lmb.c > >index a7ecbb58831f..c7bf5120696f 100644 > >--- a/lib/lmb.c > >+++ b/lib/lmb.c > >@@ -36,10 +36,10 @@ DECLARE_GLOBAL_DATA_PTR; > > static long lmb_addrs_overlap(phys_addr_t base1, phys_size_t size1, > > phys_addr_t base2, phys_size_t size2) > > { > >- const phys_addr_t base1_end = base1 + size1 - 1; > >- const phys_addr_t base2_end = base2 + size2 - 1; > >+ const phys_addr_t base1_end = base1 + size1; > >+ const phys_addr_t base2_end = base2 + size2; > > > >- return ((base1 <= base2_end) && (base2 <= base1_end)); > >+ return ((base1 < base2_end) && (base2 < base1_end)); > > } > > > > static long lmb_addrs_adjacent(phys_addr_t base1, phys_size_t size1, >
On Sun, 8 Dec 2024 at 14:23, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > Hi Heinrich > > On Sun, 8 Dec 2024 at 13:31, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > Am 8. Dezember 2024 11:52:05 MEZ schrieb Ilias Apalodimas <ilias.apalodimas@linaro.org>: > > >There's no point subtracting -1 from the calculated addresses and then > > >check for a <= b. Just remove the -1 and check for a < b. > > > > I once thought that, too. But it makes a difference for end= U(L)LONG_MAX. > > If those addresses overflow then we have to add more checks -- e.g > if (base2_end < base2) > { > Do something wrong > } > > In which case, I prefer modifying the function and add those checks > > Thoughts? > /Ilias I'm replying to myself here, but the check above isn't needed it's already implied by the return checks. I'll drop this in v2 Cheers /Ilias > > > > > Best regards > > > > Heinrich > > > > > > > >Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > >--- > > > lib/lmb.c | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > >diff --git a/lib/lmb.c b/lib/lmb.c > > >index a7ecbb58831f..c7bf5120696f 100644 > > >--- a/lib/lmb.c > > >+++ b/lib/lmb.c > > >@@ -36,10 +36,10 @@ DECLARE_GLOBAL_DATA_PTR; > > > static long lmb_addrs_overlap(phys_addr_t base1, phys_size_t size1, > > > phys_addr_t base2, phys_size_t size2) > > > { > > >- const phys_addr_t base1_end = base1 + size1 - 1; > > >- const phys_addr_t base2_end = base2 + size2 - 1; > > >+ const phys_addr_t base1_end = base1 + size1; > > >+ const phys_addr_t base2_end = base2 + size2; > > > > > >- return ((base1 <= base2_end) && (base2 <= base1_end)); > > >+ return ((base1 < base2_end) && (base2 < base1_end)); > > > } > > > > > > static long lmb_addrs_adjacent(phys_addr_t base1, phys_size_t size1, > >
diff --git a/lib/lmb.c b/lib/lmb.c index a7ecbb58831f..c7bf5120696f 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -36,10 +36,10 @@ DECLARE_GLOBAL_DATA_PTR; static long lmb_addrs_overlap(phys_addr_t base1, phys_size_t size1, phys_addr_t base2, phys_size_t size2) { - const phys_addr_t base1_end = base1 + size1 - 1; - const phys_addr_t base2_end = base2 + size2 - 1; + const phys_addr_t base1_end = base1 + size1; + const phys_addr_t base2_end = base2 + size2; - return ((base1 <= base2_end) && (base2 <= base1_end)); + return ((base1 < base2_end) && (base2 < base1_end)); } static long lmb_addrs_adjacent(phys_addr_t base1, phys_size_t size1,
There's no point subtracting -1 from the calculated addresses and then check for a <= b. Just remove the -1 and check for a < b. Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> --- lib/lmb.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)