Re: [PATCH 3/4] EDAC: Refactor edac_align_ptr() to use u8/u16/u32/u64 data types

From: Borislav Petkov
Date: Tue Feb 15 2022 - 11:55:16 EST


On Thu, Jan 13, 2022 at 10:06:21AM +0000, Eliav Farber wrote:
> Prefer well defined size variables, that are same in size across all
> systems.
>
> Signed-off-by: Eliav Farber <farbere@xxxxxxxxxx>
> ---
> drivers/edac/edac_mc.c | 17 ++++++-----------
> 1 file changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
> index 8b9b86a7720a..3367bf997b73 100644
> --- a/drivers/edac/edac_mc.c
> +++ b/drivers/edac/edac_mc.c
> @@ -250,18 +250,13 @@ void *edac_align_ptr(void **p, unsigned size, int n_elems)
> * 'size'. Adjust 'p' so that its alignment is at least as
> * stringent as what the compiler would provide for X and return
> * the aligned result.
> - * Here we assume that the alignment of a "long long" is the most
> - * stringent alignment that the compiler will ever provide by default.
> - * As far as I know, this is a reasonable assumption.
> */
> - if (size > sizeof(long))
> - align = sizeof(long long);
> - else if (size > sizeof(int))
> - align = sizeof(long);
> - else if (size > sizeof(short))
> - align = sizeof(int);
> - else if (size > sizeof(char))
> - align = sizeof(short);
> + if (size > sizeof(u32))
> + align = sizeof(u64);
> + else if (size > sizeof(u16))
> + align = sizeof(u32);
> + else if (size > sizeof(u8))
> + align = sizeof(u16);
> else
> return ptr;

This is just silly. I think you should simply align on 8 and kill all
that bla.

This whole pointer alignment, then picking out the actual pointers of
the embedded struct members is just a bunch of unneeded complexity. I'd
like to get rid of it completely one day...

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette