[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJ-ks9=DMvchc9ZR0bdP4Q3c3cJX2Qx7SyPjN9wgHjH3ht_2DA@mail.gmail.com>
Date: Thu, 6 Feb 2025 13:51:59 -0500
From: Tamir Duberstein <tamird@...il.com>
To: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
Cc: Danilo Krummrich <dakr@...nel.org>, Miguel Ojeda <ojeda@...nel.org>,
Alex Gaynor <alex.gaynor@...il.com>, Boqun Feng <boqun.feng@...il.com>,
Gary Guo <gary@...yguo.net>, Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <benno.lossin@...ton.me>, Andreas Hindborg <a.hindborg@...nel.org>,
Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>, rust-for-linux@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] rust: alloc: satisfy `aligned_alloc` requirements
On Thu, Feb 6, 2025 at 1:23 PM Miguel Ojeda
<miguel.ojeda.sandonis@...il.com> wrote:
>
> On Thu, Feb 6, 2025 at 7:11 PM Tamir Duberstein <tamird@...il.com> 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
Powered by blists - more mailing lists