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

Wolfgang Denk
2010-05-19T15:14:02Z

Dear Timur Tabi,

In message <4BF3FC6D.4000605@freescale.com> you wrote:
>
> But I believe I have already fixed it by stating that any users of
> fdt_increase_size() must ensure that the new size fits in the allocated
> area. The same rules apply to functions like strcat(), so I don't
> understand why you are so concerned.

I think the places where strcat() is used actually try and make sure
to have enough room in the target buffer; also, almost all of themn
appent static strigs with well-known sizes only.

You talked about inserting user-supplied data of unknown size.

This is a completely different thing.

> But there is no way to ensure that every user of fdt_increase_size() will
> follow those rules.

There is always a way if you really try.

> I could, for instance, add an lmb parameter to fdt_increase_size(), but this
> will only apply in instances where the fdt exists inside an lmb. There is no
> lmb_realloc() function, so the most I can do is return an error if the lmb
> isn't big enough.

You could add a lmb_realloc() function...

> The problem is that by the time the code needs to resize the fdt, it has
> long since forgotten about the lmb. We would need to modify

You could add code so that this information gets stored in a suitable
way.

> ft_board_setup() to take an lmb as a parameter, but that would require
> changes to almost 30 boards!

There have been changes in the past that required changes to many,
many more boards. What exactly is the problem with doing this?

> And then, that doesn't help the situation where the fdt is just dumped in
> memory somewhere, like this:

Right. In this case the user is responsible. But that's an unrelated
use case.

> So if an lmb is provided, we can check for overwrites, but we can't require
> the lmb.

Eventually you can do something like lmb_realloc() as well.

> The only problem with the above function is that lmb_size() doesn't exist,

Then add it?

> and I'm not sure how to implement it yet. There doesn't appear to be any
> useful support functions for the lmb code at the moment.

Probably there was no need for these yet. Now there is.

> > 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.
>
> Actually, I can. For firmware, at least, I have macro called
> CONFIG_SYS_FSL_FW_MAXSIZE that defines the largest size that firmware can

Why would you add another static buffer size to excuse the limitations
of another one?

Please expect me to reject a patch that adds CONFIG_SYS_FSL_FW_MAXSIZE
with the same argumets we're discussing here.

> applicable), but to also ensure limits on the firmware itself. This would
> be a board-specific setting, since each board knows what kind of firmware it
> needs. And then I have this:

A "board" cannot ever know what the user will be doing.

> Thus, each board can define its own settings, and boot_relocate_fdt() will
> honor them.

This makes little or no sense to me.

When we load such firmware from some form of image, we know exactly
what size if has. And we have mechanisms in place to use dynamically
sized buffers. Use these, please.

> > I will not accept code that relies on such compile time assumptions
> > about dynamically inserted data.
>
> I don't see any way around that.

You know my opinion.

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
"There's only one kind of woman ..." "Or man, for that matter. You
either believe in yourself or you don't."
-- Kirk and Harry Mudd, "Mudd's Women", stardate 1330.1