Re: [PATCH v2 6/8] ACPI: property: Unify integer value reading functions

Sakari Ailus
2022-05-20T22:05:04Z

Hi Andy,

Thanks for the review.

On Fri, May 20, 2022 at 05:25:09PM +0300, Andy Shevchenko wrote:
> On Fri, May 20, 2022 at 09:11:46AM +0300, Sakari Ailus wrote:
> > Unify functions reading ACPI property integer values into a single macro,
> > and call that macro to generate the functions for each bit depth.
> >
> > Also use size_t for the counter instead of int.
>
> ...
>
> > +#define DECLARE_ACPI_PROPERTY_COPY(bits) \
> > + static int \
> > + acpi_copy_property_array_u##bits(const union acpi_object *items, \
>
> Personally I find much better if function templates in macros are not indented
> additionally.

I prefer it the way it is in this patch: indenting the macro has grounds as
it is part of the same pre-processor directive even if it's split on
multiple lines.

>
> > + u##bits *val, size_t nval) \
> > + { \
> > + size_t i; \
> > + \
> > + for (i = 0; i < nval; i++) { \
> > + if (items[i].type != ACPI_TYPE_INTEGER) \
> > + return -EPROTO; \
> > + if (items[i].integer.value > U##bits##_MAX) \
> > + return -EOVERFLOW; \
> > + \
> > + val[i] = items[i].integer.value; \
> > + } \
> > + return 0; \
> > }
>
> On top of that, we use a minimum compiler that supports _Generic(). Why not to
> use it?

Good idea, I'll look into this. Perhaps we'll even get rid of having to
define multiple functions for this.

--
Kind regards,

Sakari Ailus