[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANiq72mzJbX+_tpqSOsWpgtAeYD0rutVJ2V6u0wDO+zGcB51Ww@mail.gmail.com>
Date: Thu, 6 Feb 2025 19:23:00 +0100
From: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
To: Tamir Duberstein <tamird@...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 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.
Thanks!
Cheers,
Miguel
Powered by blists - more mailing lists