Re: [PATCH v2 04/10] libfdt: overlay: Factor out fixup and fragment path parsing helpers

[Doug Anderson]

Hi David,

On Thu, Oct 29, 2026 at 8:55 PM David Gibson
<david@xxxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Wed, Oct 28, 2026 at 02:55:54PM -0700, Douglas Anderson wrote:
> > Extract `overlay_parse_fixup_entry()` from `overlay_fixup_phandle()` and
> > `overlay_split_fragment_path()` from `overlay_symbol_update()`.
> >
> > A subsequent patch updating the base tree's `/__local_fixups__` node
> > during `fdt_overlay_apply()` needs to parse the same `"path:name:offset"`
> > entries in `/__fixups__` and split `/<fragment>/__overlay__/<subpath>`
> > paths in the same way.
> >
> > No functional change intended.
>
> Conceptually good, but has a couple of warts.
>
> > +/**
> > + * overlay_parse_fixup_entry - Parse one "path:name:offset" entry from /__fixups__
> > + * @valuep: In/out pointer to the current NUL-terminated fixup string
> > + * @lenp: In/out remaining length of the /__fixups__ property value
>
> It would be good to describe how the in/out values are updated.

Done. Updated the doc comment to specify that `@valuep` is advanced past
the terminating NUL byte to the next entry and `@lenp` is decremented by
the consumed entry length (including its NUL terminator).


> > +static int overlay_split_fragment_path(const char *path, int path_len,
> > +                                    const char **frag_namep,
> > +                                    int *frag_name_lenp,
> > +                                    const char **rel_pathp,
> > +                                    int *rel_path_lenp,
> > +                                    bool *is_overlayp)
>
> Returning the bool this way is clunky.  Better to use the return value.

Done. Dropped `bool *is_overlayp` and updated `overlay_split_fragment_path()`
to return `1` if `@path` is under an `__overlay__` subnode, `0` if it is a
valid path not under `__overlay__`, or `-FDT_ERR_BADVALUE` on error.


-Doug