Re: [PATCH v2] rust: alloc: satisfy `aligned_alloc` requirements

From: Tamir Duberstein
Date: Thu Feb 06 2025 - 13:57:15 EST


On Thu, Feb 6, 2025 at 1:23 PM Miguel Ojeda
<miguel.ojeda.sandonis@xxxxxxxxx> wrote:
>
> On Thu, Feb 6, 2025 at 7:11 PM Tamir Duberstein <tamird@xxxxxxxxx> wrote:
> >
> > The note on cppreference addresses this:
> >
> > > As an example of the "supported by the implementation" requirement, POSIX
> > > function posix_memalign accepts any alignment that is a power of two and a
> > > multiple of sizeof(void *), and POSIX-based implementations of aligned_alloc
> > > inherit this requirements.
>
> Yes, at least some POSIX-based implementation inherit some
> requirements, but the commit talks about the "documented requirements"
> of `alligned_alloc`, which didn't seem right to me (in fact, some
> implementations seem to be extremely lax (e.g. glibc)).
>
> > I could rework this patch to use posix_memalign which seems to be more
> > completely defined, or I can try to capture all this detail in a code
> > comment and the commit message. What do you folks prefer?
>
> I suggested going with that "macOS" etc. line because that is what we
> are doing and so that we avoid having to put a lot of complexity in
> that comment.
>
> In other words, we are changing it so that it works in macOS, right?
> And those requirements seem the stricter ones vs. say glibc ones. So
> going with the "why we changed this" may be an easier way to explain
> why we are actually changing this, unless we are sure we know those
> are the requirements for everyone (which is what the current comment
> in the code looks like).
>
> But if using another function makes it clearer, that is great too.
>
> In any case, to be clear, I didn't want to delay the change -- it is
> just that the commit message and the comment didn't seem correct.

You're absolutely correct, of course. I've changed this to use
`posix_memalign` so that the behavior is identical for everyone. I'll
send v3 shortly.

> Thanks!
>
> Cheers,
> Miguel

Thank you!
Tamir