Re: [XEN][PATCH v3 1/1] Update libfdt to v1.6.1

[Julien Grall <julien@xen.org>]

Hi Vikram,

On 05/11/2021 21:28, Vikram Garhwal wrote:
> Update libfdt to v1.6.1 of libfdt taken from git://github.com/dgibson/dtc.
> This update is done to support device tree overlays.
> 
> A few minor changes are done to make it compatible with Xen:
>      fdt_overlay.c: overlay_fixup_phandle()
> 
>          Replace strtoul() with simple_strtoul() as strtoul() is not available in
>          Xen lib and included lib.h.
> 
>          Change char *endptr to const char *endptr.

I would clarify this is a requirement to use simple_strtoul().

[...]

> diff --git a/xen/include/xen/libfdt/libfdt_env.h b/xen/include/xen/libfdt/libfdt_env.h
> index 035bf75..03380d5 100644
> --- a/xen/include/xen/libfdt/libfdt_env.h
> +++ b/xen/include/xen/libfdt/libfdt_env.h
> @@ -1,22 +1,98 @@
> -#ifndef _LIBFDT_ENV_H
> -#define _LIBFDT_ENV_H
> +/* SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause) */
> +#ifndef LIBFDT_ENV_H
> +#define LIBFDT_ENV_H
> +/*
> + * libfdt - Flat Device Tree manipulation
> + * Copyright (C) 2006 David Gibson, IBM Corporation.
> + * Copyright 2012 Kim Phillips, Freescale Semiconductor.
> + */
>   
> +#include <xen/config.h>

The build system should always include <xen/config.h> automagically. So 
can you clarify why this is necessary?

>   #include <xen/types.h>
>   #include <xen/string.h>
>   #include <asm/byteorder.h>
> +#include <xen/stdbool.h>

We usually order the headers so <xen/...> are first and then <asm/...>. 
They are then ordered alphabetically within themself.

But it is not clear why you are adding <xen/stdbool.h> as AFAICT it is 
already going to be included by existing headers. Did you see an error 
without including it?

Cheers,

-- 
Julien Grall