[U-Boot] libfdt: make fdt_increase_size() available to everyone

[Wolfgang Denk]

Dear Timur Tabi,

In message <AANLkTimFF-MM8jLicndervkWwRVpMQEeEuVd_RNTMxrY at mail.gmail.com> you wrote:
> 
> >> And again, the point *I* am trying to make is that it's okay to put the onus
> >> of that check on the *caller* of fdt_increase_size(), and not on
> >> fdt_increase_size() itself.
> >
> > OK. I have no problem with that. I am just missing this other part of
> > the required changes in your patch.
>
> I'm not ready to submit any code that calls fdt_increase_size() yet.
> I'm just trying to create the infrastructure here so that I can be
> sure that my in-house code is correct.  If this patch is rejected
> because there's a better way to increase the size of an fdt, then I
> need to know that *now*.

I reject your patch because it introduces the risk of crashing the
system and it appears you do not want to be bothered fixing this.

> > Maybe. But there is still case #1, where we have a real problem, and I
> > will not accept your patch without seing this problem properly
> > addressed.
>
> And I said that we'll handle this by setting a new value of
> CONFIG_SYS_FDT_PAD.  This will ensure that the initial memory
> allocation is large enough.

And I said that this is not sufficient. Static buffers have always
been a bad idea as there will always be a user who needs more space.
In this case, where we know the amount needed, we can as well arrange
for it. You may have to add some code to implemenmt this, butthen
adding this code has to be part of your patch submission.

> Technically speaking, even without my patch we already have the
> problem that bothers you.  When boot_relocate_fdt() allocates the LMB,
> it gives it an additional buffer of 12KB.  There's nothing stopping
> some code from calling fdt_setprop() and/or fdt_add_subnode()  a
> thousand times, which will cause the fdt to grow outside of the
> allocated area.

Right. We don't provide safety belts against doing stupid things. But
you mentioned your extension is needed to insert firmware blobs which
can take "significant size" - you cannot estimate in advace (at
compile time) which size the end user may need. So using a static
buffer size is a stupid thing too do as is is bound to fail rather
sooner than later.

I will not accept code that relies on such compile time assumptions
about dynamically inserted data.

[Note that it does not help that *you* *shout* *at* *me* all the time
that you need this *now*. This does not make your patch any better.]


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If ignorance is bliss, why aren't there more happy people?