Re: [PATCH 3/4] dt-overlay: Remove ASSERT_UNREACHABLE from add_nodes()

[Julien Grall <julien@xxxxxxx>]

On 23/09/2024 11:44, Michal Orzel wrote:
> Hi Julien,
>
> On 20/09/2024 10:35, Julien Grall wrote:
> > Hi Michal,
> >
> > On 19/09/2024 12:42, Michal Orzel wrote:
> > > The assumption stated in the comment that the code will never get there
> > > is incorrect. It's enough for the target-path to be incorrect (i.e. user
> > > error), which will lead to an incorrect overall node path and we will end
> > > up in this "unreachable" place causing a failure in debug builds.
> > Looking at the caller, nodes_full_path contain path that was computed
> > from the overlay. So I would have assumed the path would be part of the
> > new DT. What did I miss?
> nodes_full_path contains paths to nodes by combining the name of the node with 
> the target-path string.
> It's generated in overlay_get_nodes_info(). It's a simple <target-path> + '/' + 
> <node-path> + '\0'. It does
> not have any dt logic as for paths. On the other hand libfdt contains advanced 
> logic and can tweak the node
> path if needed. So for example if the target-path is "////" and node path is "foo@0", 
> libfdt full path will be "/foo@0"
> (notice how it got rid of excessive slashes) but our nodes_full_path[foo] will be 
> "////foo@0". The only place which can
> spot this difference is our check in add_nodes().
Thanks for the clarification. This reads as we want to get the list of 
nodes using the libfdt logic rather than re-inventing our own.
I would suggest to expand a bit more the commit message with the details 
you provided. With that:
Acked-by: Julien Grall <jgrall@xxxxxxxxxx>

Cheers,

--
Julien Grall