On Thu, 27 Jul 2023 at 19:51, Simon Glass <[email protected]> wrote: > > Hi Dan, > > On Thu, 27 Jul 2023 at 04:56, Dan Carpenter <[email protected]> wrote: > > > > On Wed, Jul 26, 2023 at 06:49:44PM -0600, Simon Glass wrote: > > > Hi Dan, > > > > > > On Tue, 25 Jul 2023 at 09:40, Dan Carpenter <[email protected]> > > > wrote: > > > > > > > > The > comparison needs to be changed to >= to prevent an out of bounds > > > > write on th next line. > > > > > > > > Signed-off-by: Dan Carpenter <[email protected]> > > > > --- > > > > lib/addr_map.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/lib/addr_map.c b/lib/addr_map.c > > > > index 9b3e0a544e47..86e932e4b561 100644 > > > > --- a/lib/addr_map.c > > > > +++ b/lib/addr_map.c > > > > @@ -59,7 +59,7 @@ void *addrmap_phys_to_virt(phys_addr_t paddr) > > > > void addrmap_set_entry(unsigned long vaddr, phys_addr_t paddr, > > > > phys_size_t size, int idx) > > > > { > > > > - if (idx > CONFIG_SYS_NUM_ADDR_MAP) > > > > + if (idx >= CONFIG_SYS_NUM_ADDR_MAP) > > > > return; > > > > > > It looks like this function should return an error. > > > > > > > If we hit this error path there probably isn't a reasonable way to > > recover. Maybe we could add a WARN()? > > When we get an error we should report it. For leaf functions like > this, WARN adds to code size for a case that should not happen. The > caller will need to check the error and fail. The function should > always have had an error-return code, by the look of it. If we adopt > the approach of warning instead of returning error codes, > > +Bin Meng > +Kumar Gala
Since this is a fix, separate from the poor API: Reviewed-by: Simon Glass <[email protected]>

