Re: [PATCH v6 2/8] Add a way to control the level of checks in the code

[David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1641 bytes --]

On Tue, Feb 11, 2020 at 01:09:39PM -0700, Simon Glass wrote:
> Add a new ASSUME_MASK option, which allows for some control over the
> checks used in libfdt. With all assumptions enabled, libfdt assumes that
> the input data and parameters are all correct and that internal errors
> cannot happen.
> 
> By default no assumptions are made and all checks are enabled.
> 
> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

LGTM, except...

[snip]
> +	/*
> +	 * This assumes that the device tree components appear in the correct
> +	 * order. As such it disables a check in fdt_open_into() and removes the
> +	 * ability to fix the problem there. This is safe if you know that the
> +	 * device tree is correctly ordered. See fdt_blocks_misordered_().
> +	 */
> +	ASSUME_CORRECT_ORDER	= 1 << 4,

.. I don't like this name.  The spec doesn't specify an order, so
having it in a different order isn't incorrect, just a bit harder to
work with.

> +};
> +
> +/**
> + * can_assume_() - check if a particular assumption is enabled
> + *
> + * @mask: Mask to check (ASSUME_...)
> + * @return true if that assumption is enabled, else false
> + */
> +static inline bool can_assume_(int mask)
> +{
> +	return FDT_ASSUME_MASK & mask;
> +}
> +
> +/** helper macros for checking assumptions */
> +#define can_assume(_assume)	can_assume_(ASSUME_ ## _assume)
> +
>  #endif /* LIBFDT_INTERNAL_H */

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]