linux-kernel - Re: [PATCH v5 04/26] rust: alloc: implement `Allocator` for `Kmalloc`

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANiq72=u5Nrz_NW3U3_VqywJkD8pECA07q2pFDd1wjtXOWdkAQ@mail.gmail.com>
Date: Wed, 14 Aug 2024 18:02:22 +0200
From: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
To: Danilo Krummrich <dakr@...nel.org>
Cc: Alice Ryhl <aliceryhl@...gle.com>, ojeda@...nel.org, alex.gaynor@...il.com, 
	wedsonaf@...il.com, boqun.feng@...il.com, gary@...yguo.net, 
	bjorn3_gh@...tonmail.com, benno.lossin@...ton.me, a.hindborg@...sung.com, 
	akpm@...ux-foundation.org, daniel.almeida@...labora.com, 
	faith.ekstrand@...labora.com, boris.brezillon@...labora.com, 
	lina@...hilina.net, mcanal@...lia.com, zhiw@...dia.com, cjia@...dia.com, 
	jhubbard@...dia.com, airlied@...hat.com, ajanulgu@...hat.com, 
	lyude@...hat.com, linux-kernel@...r.kernel.org, 
	rust-for-linux@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH v5 04/26] rust: alloc: implement `Allocator` for `Kmalloc`

On Wed, Aug 14, 2024 at 5:20 PM Danilo Krummrich <dakr@...nel.org> wrote:
>
> Indeed, and I think once they're honored we should add them again.
>
> It's just that I think as long as compiler attributes aren't honored, we should
> not have them in the first place to avoid confusion about whether they do or do
> not have any effect.

In the C side, many attributes are not honored anyway, depending on
the compiler (or tool like sparse), compiler version, etc. So it would
be "OK" in that sense (even if here we may know there is no C caller).

> It's not so much that I want to remove them for krealloc(), it's that I don't
> want to intentionally add them for the vrealloc() and kvrealloc() helpers,
> knowing that they don't do anything (yet).

One can think of them as "documentation", e.g. seeing `__must_check`
tells you something about the function, so I don't think it would be
bad to add them, if they are not too much work to maintain.

I checked about `__must_check`, because it would be nice if it is used
by `bindgen`, and it turns out it already does, but behind
`--enable-function-attribute-detection` (apparently for performance
reasons):

    int f(void);
    __attribute__((warn_unused_result)) int g(void);

gives:

    extern "C" {
        pub fn f() -> ::std::os::raw::c_int;
    }
    extern "C" {
        #[must_use]
        pub fn g() -> ::std::os::raw::c_int;
    }

So that one should be kept at the very least, and it is a good example
of potential improvements later on based on these attributes.

In general, I think it is not a bad idea to write this file like we
would write other C files, even if it is not a regular C file. And
adding more information to signatures is, after all, part of what we
do with Rust overall!

Cheers,
Miguel