Hi Quentin,

On Mon, 27 Apr 2026 at 03:14, Quentin Schulz <[email protected]> wrote:
>
> Hi Simon, Marek,
>
> On 4/22/26 4:31 AM, Simon Glass wrote:
> > When producing a DTB from fdtgrep, there is no way to reserve extra
> > space so that more nodes/properties can be added at runtime without
> > growing totalsize first.
> >
>
> Ok but why is that an issue? What is the actual usecase? What I'm afraid
> of is that you pad enough and you hit random bugs because you forget
> that adding enough properties can now hit the size limit and you forgot
> to add the logic to resize/grow the fdt when it's too small to fit new
> properties.

I'm going to leave this question to Marek :-)

>
> > Add a -B/--pad option that appends a caller-specified number of free
> > bytes after the strings section and grows totalsize accordingly. The
> > padding is applied after fdt_pack(), so it is not reclaimed when -r is
> > used.
> >
> > Suggested-by: Marek Vasut <[email protected]>
> > Signed-off-by: Simon Glass <[email protected]>
> > ---
> >
> >   tools/binman/btool/fdtgrep.py |  3 ++-
> >   tools/fdtgrep.c               | 31 ++++++++++++++++++++++++++++++-
> >   2 files changed, 32 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/binman/btool/fdtgrep.py b/tools/binman/btool/fdtgrep.py
> > index 446b2f4144b..7b393241878 100644
> > --- a/tools/binman/btool/fdtgrep.py
> > +++ b/tools/binman/btool/fdtgrep.py
> > @@ -16,9 +16,10 @@ Output formats are:
> >       dtb - device tree blob (sets -Hmt automatically)
> >       bin - device tree fragment (may not be a valid .dtb)
> >
> > -Options: -[haAc:b:C:defg:G:HIlLmn:N:o:O:p:P:rRsStTvhV]
> > +Options: -[haAB:c:b:C:defg:G:HIlLmn:N:o:O:p:P:rRsStTvhV]
> >     -a, --show-address                 Display address
> >     -A, --colour                       Show all nodes/tags, colour those 
> > that match
> > +  -B, --pad <arg>                    Append free space to the output FDT, 
> > for later growth
> >     -b, --include-node-with-prop <arg> Include contains containing property
> >     -c, --include-compat <arg>         Compatible nodes to include in grep
> >     -C, --exclude-compat <arg>         Compatible nodes to exclude in grep
>
> I'm tempted to remove this and just tell people to look at the
> documentation in tools/fdtgrep.c. Especially since we don't actually
> allow the user to pass any of those parameters themselves via the
> bintool, so it isn't actually useful.

I would like fdtgrep to give useful help, though. It is a tool which
can stand alone.

>
> > diff --git a/tools/fdtgrep.c b/tools/fdtgrep.c
> > index b4c041070f5..83e3d0b9314 100644
> > --- a/tools/fdtgrep.c
> > +++ b/tools/fdtgrep.c
> > @@ -64,6 +64,7 @@ struct display_info {
> >       int types_exc;          /* Mask of types that we exclude (FDT_IS...) 
> > */
> >       int invert;             /* Invert polarity of match */
> >       int props_up;           /* Imply properties up to supernodes */
> > +     int pad;                /* Free bytes to append after the strings 
> > section */
> >       struct value_node *value_head;  /* List of values to match */
> >       const char *output_fname;       /* Output filename */
> >       FILE *fout;             /* File to write dts/dtb output */
> > @@ -913,6 +914,27 @@ static int do_fdtgrep(struct display_info *disp, const 
> > char *filename)
> >                       size = fdt_totalsize(fdt);
> >               }
> >
> > +             if (disp->pad) {
> > +                     int new_size = fdt_totalsize(fdt) + disp->pad;
> > +                     void *new_fdt = realloc(fdt, new_size);
> > +
> > +                     if (!new_fdt) {
> > +                             fprintf(stderr, "Out_of_memory\n");
>
> Should we free fdt here?
>
> man realloc says
>
>         If there is not enough available memory, realloc() shall return a
>         null pointer and set errno to [ENOMEM].  If realloc() returns a
>         null pointer and errno has been set to [ENOMEM], the memory
>         referenced by ptr shall not be changed.
>
> though it also says
>
>         The realloc() function shall deallocate the old object pointed to
>         by ptr and return a pointer to a new object that has the size
>         specified by size.
>
> so if it happens in that order, it's freed before being reallocated (but
> I would guess this to be racey since you want to copy the content of the
> old object into the new object and if you free the space and it can be
> claimed and used by something else).

My understanding is that it does not touch fdt on failure, so we need
free() as you say.

I'll hold off on v2 until you and Marek have figured out the other
questions / issues, since they are bigger than fdtgrep itself.

Regards,
Simon

Reply via email to