Re: POINTER_ALIGNED_P (was: Re: CVS commit: src/sys)

[Valery Ushakov <uwe%stderr.spb.ru@localhost>]

On Tue, Feb 16, 2021 at 23:54:46 +0100, Roland Illig wrote:

> On 16.02.2021 23:40, Valery Ushakov wrote:
> > On Tue, Feb 16, 2021 at 22:56:19 +0100, Roland Illig wrote:
> > 
> > > On 16.02.2021 19:46, Roy Marples wrote:
> > > > I suggest we change POINTER_ALIGNED_P to accept the alignment value we
> > > > want rather than the bitwise test we supply, like so:
> > > > 
> > > > #define POINTER_ALIGNED_P(p, a) (((uintptr_t)(p) & ((a) - 1)))
> > > 
> > > To make sure that this macro doesn't get broken again, I have written
> > > the attached tests.  I will commit them after making sure I got the
> > > changes to distrib/sets/lists/tests/mi correct.  All the rest I already
> > > tested successfully.
> > 
> > I don't see any proposal to change the meaning of this macro to
> > actually require the alignment even for arches without strict
> > alignment.  Does the attached test really passes on e.g. x86 where the
> > macro is always true?  E.g. this one:
> > 
> > > +	if (POINTER_ALIGNED_P(p + 1, 2))
> > > +		atf_tc_fail("p + 1 must not be aligned to 2");
> 
> Yes, it does.  That's what the "#undef __NO_STRICT_ALIGNMENT" in the
> test is for.
> 
> I intentionally placed it between <sys/types.h> (which defines that
> macro on x86 and some other platforms) and <sys/param.h> (which uses the
> macro to switch between the boring "everything is correctly aligned" and
> the more interesting formula suggested here.

This is wrong on so many levels.  What is even the point of a test
that doesn't test the thing as it is actually defined and used in the
code?

-uwe