Re: [PATCH v2 08/15] xen/dt: Move bootfdt functions to xen/bootfdt.h

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 9 Jun 2025, Alejandro Vallejo wrote:
> On Fri Jun 6, 2025 at 9:59 PM CEST, Stefano Stabellini wrote:
> > On Fri, 6 Jun 2025, Alejandro Vallejo wrote:
> >> On Fri Jun 6, 2025 at 10:59 AM CEST, Michal Orzel wrote:
> >> >
> >> >
> >> > On 05/06/2025 21:48, Alejandro Vallejo wrote:
> >> >> Part of an unpicking process to extract bootfdt contents independent of 
> >> >> bootinfo
> >> >> to a separate file for x86 to take.
> >> >> 
> >> >> Move functions required for early FDT parsing from device_tree.h and 
> >> >> arm's
> >> >> setup.h onto bootfdt.h
> >> >> 
> >> >> Declaration motion only. Not a functional change.
> >> >> 
> >> >> Signed-off-by: Alejandro Vallejo <agarciav@xxxxxxx>
> >> >> ---
> >> >> v2:
> >> >>   * Remove the u32 identifiers in the device_tree_get_u32() 
> >> >> implementation
> >> > I don't understand the reasoning behind changing u32->uint32_t only for 
> >> > one
> >> > function in this patch while leaving others unmodified. Also what about 
> >> > u64?
> >> > Either don't change any or change all.
> >> 
> >> Sure. Let's call the original u32->uint32_t change a misplaced mutation and
> >> move on. The point is the motion, not these cleanups on top.
> >
> > Yes I agree. I know from past experience that Jan doesn't mind changes
> > during code movements, but for me it is important that changes and code
> > movement are separate. That is because I have almost automatic ways to
> > check that code movement is correct if there are no changes. It saves me
> > a lot of time during review. Then I can look at the individual changes
> > separately.
> 
> That's interesting. Could you please share the runes? That's one side of
> review I still struggle with.

I use vim to generate two source files, one with the code corresponding
to the - lines (with the - prefix) and one with the code corresponding
to the + lines (without the + prefix). Then I diff the two files. This
method is a bit crude but very effective and managed to spot issues this
way more than once.