Re: [PATCH 11/13] dtc: Add helpers for the errors when merging a fragment

[Herve Codina <herve.codina@bootlin.com>]

On Wed, 4 Nov 2026 17:49:09 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Tue, Nov 03, 2026 at 06:23:44PM +0100, Herve Codina wrote:
> > On Sun,  1 Nov 2026 09:37:05 -0700
> > Simon Glass <sjg@chromium.org> wrote:
> >   
> > > Four of the merge functions look up the node which a fragment refers to
> > > and report 'Label or path not found' if it is missing, and three check
> > > for a label-relative reference in a plugin. Each of these has its own
> > > copy of the error, so a change to a message or its position must be made
> > > in several places.
> > > 
> > > Add lookup_ref() to look up the node, reporting an error if it is not
> > > found, and check_relative_ref() to report a label-relative reference.
> > > Use these in the merge functions.
> > > 
> > > The output does not change.
> > > 
> > > Assisted-by: Claude:claude-opus-5-5
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > ---
> > > 
> > >  livetree.c | 90 +++++++++++++++++++++++++++++-------------------------
> > >  1 file changed, 49 insertions(+), 41 deletions(-)
> > > 
> > > diff --git a/livetree.c b/livetree.c
> > > index 6a290fb..1e012d4 100644
> > > --- a/livetree.c
> > > +++ b/livetree.c
> > > @@ -548,6 +548,40 @@ static bool is_ref_relative(const char *ref)
> > >  	return ref[0] != '/' && strchr(&ref[1], '/');
> > >  }
> > >  
> > > +/*
> > > + * Report an error at pos if a fragment uses a label-relative reference, which
> > > + * a plugin does not support
> > > + *
> > > + * Returns true if there is an error
> > > + */
> > > +static bool check_relative_ref(struct fragment *f, struct srcpos *pos)
> > > +{
> > > +	if (!is_ref_relative(f->ref))
> > > +		return false;
> > > +	srcpos_error(pos, "Error",
> > > +		     "Label-relative reference %s not supported in plugin",
> > > +		     f->ref);  
> > 
> > You suppose that srcpos is valid and so this function and remaining ones are
> > be called only from dt_from_source() with the parsing done by dtc-parser.y.
> > 
> > Not sure that this is a good assumption for the future.
> > 
> > IHMO, it will be better to decouple those functions from the type of parsing
> > or just add a simple 'if (pos)' before calling srcpos_error().  
> 
> It's a good point that eventually we'll want to get fragments from
> dtbos as well as from source, in which case we won't have srcpos
> information.  But when we are coming from source, the position
> information is valuable.
> 
> This case of maybe-present srcpos information is common enough, that
> the way to address it might be to alter srcpos_string() to accept
> NULL, returning "<unknown>" or some other suitable placeholder when
> srcpos information isn't available.

"<unknown>" or just an empty string "" to avoid any user confusion.
That's a detail.

Yes, checking pos at the low level srcpos_string() is a good things to do.

Best regards
Hervé