Re: [PATCH 2/2] hvmloader: do not include system headers for type declarations

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.02.2021 11:26, Roger Pau Monne wrote:
> --- /dev/null
> +++ b/tools/firmware/hvmloader/types.h
> @@ -0,0 +1,47 @@
> +#ifndef _HVMLOADER_TYPES_H_
> +#define _HVMLOADER_TYPES_H_
> +
> +typedef unsigned char uint8_t;
> +typedef signed char int8_t;
> +
> +typedef unsigned short uint16_t;
> +typedef signed short int16_t;
> +
> +typedef unsigned int uint32_t;
> +typedef signed int int32_t;
> +
> +typedef unsigned long long uint64_t;
> +typedef signed long long int64_t;

I wonder if we weren't better of not making assumptions on
short / int / long long, and instead use
__attribute__((__mode__(...))) or, if available, the compiler
provided __{,U}INT*_TYPE__.

> +#define INT8_MIN        (-0x7f-1)
> +#define INT16_MIN       (-0x7fff-1)
> +#define INT32_MIN       (-0x7fffffff-1)
> +#define INT64_MIN       (-0x7fffffffffffffffll-1)
> +
> +#define INT8_MAX        0x7f
> +#define INT16_MAX       0x7fff
> +#define INT32_MAX       0x7fffffff
> +#define INT64_MAX       0x7fffffffffffffffll
> +
> +#define UINT8_MAX       0xff
> +#define UINT16_MAX      0xffff
> +#define UINT32_MAX      0xffffffffu
> +#define UINT64_MAX      0xffffffffffffffffull

At least if going the above outlined route, I think we'd then
also be better off not #define-ing any of these which we don't
really use. Afaics it's really only UINTPTR_MAX which needs
providing.

> +typedef uint32_t size_t;

Like the hypervisor, we should prefer using __SIZE_TYPE__
when available.

> +typedef uint32_t uintptr_t;

Again - use __UINTPTR_TYPE__ or, like Xen,
__attribute__((__mode__(__pointer__))).

> +#define bool _Bool
> +#define true 1
> +#define false 0
> +#define __bool_true_false_are_defined   1
> +
> +typedef __builtin_va_list va_list;
> +#define va_copy(dest, src)    __builtin_va_copy((dest), (src))
> +#define va_start(ap, last)    __builtin_va_start((ap), (last))

Nit: Perhaps better omit the unnecessary inner parentheses?

Jan