Hi David,
On Tue, 28 Jan 2020 at 01:37, David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Tue, Nov 12, 2019 at 06:54:21PM -0700, Simon Glass wrote:
> > Allow enabling FDT_ASSUME_LATEST to disable version checks.
> >
> > Signed-off-by: Simon Glass <sjg@xxxxxxxxxxxx>
>
> LGTM, one minor comment below.
>
> > ---
> >
> > Changes in v4: None
> > Changes in v3: None
> > Changes in v2: None
> >
> > libfdt/fdt.c | 34 +++++++++++++++++++++-------------
> > libfdt/fdt_ro.c | 16 ++++++++--------
> > libfdt/fdt_rw.c | 6 +++---
> > 3 files changed, 32 insertions(+), 24 deletions(-)
> >
> > diff --git a/libfdt/fdt.c b/libfdt/fdt.c
> > index d89c0e0..78dbe58 100644
> > --- a/libfdt/fdt.c
> > +++ b/libfdt/fdt.c
> > @@ -21,10 +21,13 @@ int32_t fdt_ro_probe_(const void *fdt)
> >
> > if (fdt_magic(fdt) == FDT_MAGIC) {
> > /* Complete tree */
> > - if (fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION)
> > - return -FDT_ERR_BADVERSION;
> > - if (fdt_last_comp_version(fdt) > FDT_LAST_SUPPORTED_VERSION)
> > - return -FDT_ERR_BADVERSION;
> > + if (!can_assume(LATEST)) {
> > + if (fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION)
> > + return -FDT_ERR_BADVERSION;
> > + if (fdt_last_comp_version(fdt) >
> > + FDT_LAST_SUPPORTED_VERSION)
> > + return -FDT_ERR_BADVERSION;
> > + }
> > } else if (fdt_magic(fdt) == FDT_SW_MAGIC) {
> > /* Unfinished sequential-write blob */
> > if (fdt_size_dt_struct(fdt) == 0)
> > @@ -72,7 +75,8 @@ size_t fdt_header_size_(uint32_t version)
> >
> > size_t fdt_header_size(const void *fdt)
> > {
> > - return fdt_header_size_(fdt_version(fdt));
> > + return can_assume(LATEST) ? FDT_V17_SIZE :
> > + fdt_header_size_(fdt_version(fdt));
> > }
> >
> > int fdt_check_header(const void *fdt)
> > @@ -81,11 +85,14 @@ int fdt_check_header(const void *fdt)
> >
> > if (fdt_magic(fdt) != FDT_MAGIC)
> > return -FDT_ERR_BADMAGIC;
> > - if ((fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION)
> > - || (fdt_last_comp_version(fdt) > FDT_LAST_SUPPORTED_VERSION))
> > - return -FDT_ERR_BADVERSION;
> > - if (fdt_version(fdt) < fdt_last_comp_version(fdt))
> > - return -FDT_ERR_BADVERSION;
> > + if (!can_assume(LATEST)) {
> > + if ((fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION)
> > + || (fdt_last_comp_version(fdt) >
> > + FDT_LAST_SUPPORTED_VERSION))
> > + return -FDT_ERR_BADVERSION;
> > + if (fdt_version(fdt) < fdt_last_comp_version(fdt))
> > + return -FDT_ERR_BADVERSION;
> > + }
> > hdrsize = fdt_header_size(fdt);
> > if (!can_assume(SANE)) {
> >
> > @@ -101,7 +108,7 @@ int fdt_check_header(const void *fdt)
> >
> > if (!can_assume(SANE)) {
> > /* Bounds check structure block */
> > - if (fdt_version(fdt) < 17) {
> > + if (!can_assume(LATEST) && fdt_version(fdt) < 17) {
>
> I wonder if we should make fdt_version() return 17 unconditionally in
> the can_assume() case, which might make some of these tests redundant.
I think that would require fdt_version() to change from being a macro,
since we cannot check assumptions in libfdt.h without including
libfdt_internal.h in libfdt.h
Is that acceptable?
Regards,
Simon
