Re: [PATCH v3 1/4] ieee1275: Move #defines into common ieee1275.h header

[Stefan Berger <stefanb@linux.ibm.com>]

On 7/30/21 11:45 AM, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
>
> Move some #defines from ieee1275.c into the common ieee1275.h
> header file. Adjust the case used in IHANDLE_INVALID to use
> proper ihandle_t.
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
>   grub-core/kern/ieee1275/ieee1275.c | 29 ++++++++++++-----------------
>   include/grub/ieee1275/ieee1275.h   |  3 +++
>   2 files changed, 15 insertions(+), 17 deletions(-)
>
> diff --git a/grub-core/kern/ieee1275/ieee1275.c b/grub-core/kern/ieee1275/ieee1275.c
> index 86f81a3c4..8fe92274d 100644
> --- a/grub-core/kern/ieee1275/ieee1275.c
> +++ b/grub-core/kern/ieee1275/ieee1275.c
> @@ -21,11 +21,6 @@
>   #include <grub/types.h>
>   #include <grub/misc.h>
>   
> -#define IEEE1275_PHANDLE_INVALID  ((grub_ieee1275_cell_t) -1)
> -#define IEEE1275_IHANDLE_INVALID  ((grub_ieee1275_cell_t) 0)
> -#define IEEE1275_CELL_INVALID     ((grub_ieee1275_cell_t) -1)
> -
> -\f
>   

I think it's safer to keep these here  because of how the sizes for the 
grub_ieee1275_cell_t are defined:

# grep -r grub_ieee1275_cell include/grub/ | grep typedef

include/grub/powerpc/ieee1275/ieee1275.h:typedef grub_uint32_t 
grub_ieee1275_cell_t;
include/grub/sparc64/ieee1275/ieee1275.h:typedef grub_uint64_t 
grub_ieee1275_cell_t;

# grep -r grub_ieee1275_phandle include/grub/ | grep typedef
include/grub/ieee1275/ieee1275.h:typedef grub_uint32_t 
grub_ieee1275_phandle_t;

# grep -r grub_ieee1275_ihandle include/grub/ | grep typedef
include/grub/ieee1275/ieee1275.h:typedef grub_uint32_t 
grub_ieee1275_ihandle_t;

# grep -r GRUB_IEEE1275_PH include/grub/| grep def
include/grub/ieee1275/ieee1275.h:#define GRUB_IEEE1275_PHANDLE_INVALID  
((grub_ieee1275_phandle_t) -1)

Using the global GRUB_IEEE1275_PHANDLE_INVALID here would probably break 
things on sparc64 if we now use 32bit '-1' rather than 64bit '-1'.


     Stefan



>   int
>   grub_ieee1275_finddevice (const char *name, grub_ieee1275_phandle_t *phandlep)
> @@ -44,8 +39,7 @@ grub_ieee1275_finddevice (const char *name, grub_ieee1275_phandle_t *phandlep)
>     if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
>       return -1;
>     *phandlep = args.phandle;
> -  if (args.phandle == IEEE1275_PHANDLE_INVALID)
> -    return -1;
> +  if (args.phandle == GRUB_IEEE1275_PHANDLE_INVALID) return -1;
>     return 0;
>   }
>   
> @@ -75,7 +69,7 @@ grub_ieee1275_get_property (grub_ieee1275_phandle_t phandle,
>       return -1;
>     if (actual)
>       *actual = (grub_ssize_t) args.size;
> -  if (args.size == IEEE1275_CELL_INVALID)
> +  if (args.size == GRUB_IEEE1275_CELL_INVALID)
>       return -1;
>     return 0;
>   }
> @@ -146,7 +140,7 @@ grub_ieee1275_get_property_length (grub_ieee1275_phandle_t phandle,
>     if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
>       return -1;
>     *length = args.length;
> -  if (args.length == IEEE1275_CELL_INVALID)
> +  if (args.length == GRUB_IEEE1275_CELL_INVALID)
>       return -1;
>     return 0;
>   }
> @@ -169,7 +163,7 @@ grub_ieee1275_instance_to_package (grub_ieee1275_ihandle_t ihandle,
>     if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
>       return -1;
>     *phandlep = args.phandle;
> -  if (args.phandle == IEEE1275_PHANDLE_INVALID)
> +  if (args.phandle == GRUB_IEEE1275_PHANDLE_INVALID)
>       return -1;
>     return 0;
>   }
> @@ -198,7 +192,7 @@ grub_ieee1275_package_to_path (grub_ieee1275_phandle_t phandle,
>       return -1;
>     if (actual)
>       *actual = args.actual;
> -  if (args.actual == IEEE1275_CELL_INVALID)
> +  if (args.actual == GRUB_IEEE1275_CELL_INVALID)
>       return -1;
>     return 0;
>   }
> @@ -227,7 +221,7 @@ grub_ieee1275_instance_to_path (grub_ieee1275_ihandle_t ihandle,
>       return -1;
>     if (actual)
>       *actual = args.actual;
> -  if (args.actual == IEEE1275_CELL_INVALID)
> +  if (args.actual == GRUB_IEEE1275_CELL_INVALID)
>       return -1;
>     return 0;
>   }
> @@ -355,7 +349,7 @@ grub_ieee1275_child (grub_ieee1275_phandle_t node,
>   
>     INIT_IEEE1275_COMMON (&args.common, "child", 1, 1);
>     args.node = node;
> -  args.result = IEEE1275_PHANDLE_INVALID;
> +  args.result = GRUB_IEEE1275_PHANDLE_INVALID;
>   
>     if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
>       return -1;
> @@ -379,7 +373,7 @@ grub_ieee1275_parent (grub_ieee1275_phandle_t node,
>   
>     INIT_IEEE1275_COMMON (&args.common, "parent", 1, 1);
>     args.node = node;
> -  args.result = IEEE1275_PHANDLE_INVALID;
> +  args.result = GRUB_IEEE1275_PHANDLE_INVALID;
>   
>     if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
>       return -1;
> @@ -459,7 +453,7 @@ grub_ieee1275_open (const char *path, grub_ieee1275_ihandle_t *result)
>     if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
>       return -1;
>     *result = args.result;
> -  if (args.result == IEEE1275_IHANDLE_INVALID)
> +  if (args.result == GRUB_IEEE1275_IHANDLE_INVALID)
>       return -1;
>     return 0;
>   }
> @@ -591,7 +585,7 @@ grub_ieee1275_claim (grub_addr_t addr, grub_size_t size, unsigned int align,
>       return -1;
>     if (result)
>       *result = args.base;
> -  if (args.base == IEEE1275_CELL_INVALID)
> +  if (args.base == GRUB_IEEE1275_CELL_INVALID)
>       return -1;
>     return 0;
>   }
> @@ -641,7 +635,8 @@ grub_ieee1275_set_property (grub_ieee1275_phandle_t phandle,
>     if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
>       return -1;
>     *actual = args.actual;
> -  if ((args.actual == IEEE1275_CELL_INVALID) || (args.actual != args.size))
> +  if ((args.actual == GRUB_IEEE1275_CELL_INVALID) ||
> +      (args.actual != args.size))
>       return -1;
>     return 0;
>   }
> diff --git a/include/grub/ieee1275/ieee1275.h b/include/grub/ieee1275/ieee1275.h
> index 73e2f4644..b963ee830 100644
> --- a/include/grub/ieee1275/ieee1275.h
> +++ b/include/grub/ieee1275/ieee1275.h
> @@ -24,6 +24,8 @@
>   #include <grub/types.h>
>   #include <grub/machine/ieee1275.h>
>   
> +#define GRUB_IEEE1275_CELL_INVALID     ((grub_ieee1275_cell_t) -1)
> +
>   struct grub_ieee1275_mem_region
>   {
>     unsigned int start;
> @@ -58,6 +60,7 @@ typedef grub_uint32_t grub_ieee1275_ihandle_t;
>   typedef grub_uint32_t grub_ieee1275_phandle_t;
>   
>   #define GRUB_IEEE1275_PHANDLE_INVALID  ((grub_ieee1275_phandle_t) -1)
> +#define GRUB_IEEE1275_IHANDLE_INVALID  ((grub_ieee1275_ihandle_t) 0)
>   
>   struct grub_ieee1275_devalias
>   {