[PATCH 1/3] dm: core: Check ofnode_to_offset() return value

[Simon Glass]

Hi Marek,

On Thu, 13 Nov 2025 at 14:57, Marek Vasut <marek.vasut at mailbox.org> wrote:
>
> On 11/13/25 8:33 PM, Simon Glass wrote:
>
> Hello Simon,
>
> >> diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
> >> index 071d998a0a5..55c2b3a10bb 100644
> >> --- a/drivers/core/ofnode.c
> >> +++ b/drivers/core/ofnode.c
> >> @@ -335,7 +335,7 @@ bool ofnode_name_eq_unit(ofnode node, const char *name)
> >>   int ofnode_read_u8(ofnode node, const char *propname, u8 *outp)
> >>   {
> >>          const u8 *cell;
> >> -       int len;
> >> +       int len, off;
> >>
> >>          assert(ofnode_valid(node));
> >>          log_debug("%s: %s: ", __func__, propname);
> >> @@ -343,8 +343,13 @@ int ofnode_read_u8(ofnode node, const char *propname, u8 *outp)
> >>          if (ofnode_is_np(node))
> >>                  return of_read_u8(ofnode_to_np(node), propname, outp);
> >>
> >> -       cell = fdt_getprop(gd->fdt_blob, ofnode_to_offset(node), propname,
> >> -                          &len);
> >> +       off = ofnode_to_offset(node);
> >> +       if (off < 0) {
> >> +               log_debug("(not valid)\n");
> >> +               return -EINVAL;
> >> +       }
> >
> > Ugh this is really horrible, sorry :-)
> >
> > Better to put a wrapper around it than add all these checks, extra
> > code size, etc. Also in many cases the ofnode is known to be valid(TM)
> > so it might just be a waste.
>
> How would you propose the wrapper should look like ?
>
> Keep in mind, not everything is fdt_getprop( ... ofnode_to_offset() ...)
> in this patch.

I was thinking of

my_fdt_getprop(fdt, node, propname, lenp)
{
if (node is not valid)
   return NULL;
return fdt_getprop();
}

What does fdt_getprop() return when the offset is -1 ?

Regards,
Simon