Hi Francois, On Sat, 16 May 2026 at 15:11, Francois Berder <[email protected]> wrote: > > Hi Simon, > > On 5/16/26 15:31, Simon Glass wrote: > > Hi Francois, > > > > On 2026-05-15T20:57:47, Francois Berder <[email protected]> wrote: > >> boot: cedit: Check ofnode_read_prop return value > >> > >> In h_read_settings, val variable could be NULL due to > >> ofnode_read_prop returning an error. This variable > >> would then be used as the src in strcpy. > >> > >> Add a NULL check after calling ofnode_read_prop. > >> > >> Signed-off-by: Francois Berder <[email protected]> > >> > >> boot/cedit.c | 2 ++ > >> 1 file changed, 2 insertions(+) > > > > Reviewed-by: Simon Glass <[email protected]> > > > >> diff --git a/boot/cedit.c b/boot/cedit.c > >> @@ -500,6 +500,8 @@ static int h_read_settings(struct scene_obj *obj, void > >> *vpriv) > >> tline = (struct scene_obj_textline *)obj; > >> > >> val = ofnode_read_prop(node, obj->name, &len); > >> + if (!val) > >> + return log_msg_ret('tline', -EINVAL); > >> if (len >= tline->max_chars) > >> return log_msg_ret('str', -ENOSPC); > >> strcpy(abuf_data(&tline->buf), val); > > > > Good catch! > > > > Please can you use -ENOENT rather than -EINVAL, as this is more > > consistent with the SCENEOBJT_MENU case a few lines below. > > > When I look at the way u-boot handles ofnode_read_prop returning NULL, I find > that it always uses the EINVAL return code instead of ENOENT. Let me know if > you still prefer that I change the error code to ENOENT.
Well it's not a big deal, but -EINVAL is used in of_to_plat methods, indicating that there was something wrong with the DT node. In this case, we are reading a config-editor definition and it is not released to a device, so trying to return a more varied assortment of error numbers is helpful for debugging. Regards, Simon
