Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
MIME-Version: 1.0
References: <20220516100401.7639-1-ojeda@kernel.org> <CAK8P3a3Z5qBQWUbPNEHhPDNX0XC8CQk+dUtC9K_=N3AXK2uHHA@mail.gmail.com>
In-Reply-To: <CAK8P3a3Z5qBQWUbPNEHhPDNX0XC8CQk+dUtC9K_=N3AXK2uHHA@mail.gmail.com>
From:   Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Date:   Wed, 18 May 2022 15:16:09 +0200
Message-ID: <CANiq72=_UAF5DRHCmW4qNt-Wi3yV6vPEjdLhBTijG_ZGgfpaTA@mail.gmail.com>
Subject: Re: [PATCH v1] binder: convert `BINDER_*` ioctl `#define`s into an `enum`
To:     Arnd Bergmann <arnd@arndb.de>
Cc:     Miguel Ojeda <ojeda@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        rust-for-linux <rust-for-linux@vger.kernel.org>,
        Masahiro Yamada <masahiroy@kernel.org>,
        Li Li <dualli@google.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Wedson Almeida Filho <wedsonaf@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, May 18, 2022 at 9:52 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> I wonder if that breaks any tools that extract ioctl command number definitions
> from kernel headers. I see one other header using enum, but everything else
> has the #define. The main user of ioctl command definitions that comes to
> mind is 'strace', but I don't know where they get the numbers from.
>
> It's probably not a big deal as long as it's limited to binder, but it
> may become
> more of a problem if we do this for more subsystems over time.

Good point. For public headers, we may just want to do this on the
Rust side, after all. And it should be as automated as possible too --
I will take a look.

In any case, I also took at `strace`. They indeed parse the C files --
with regexes, not an actual C preprocessor/compiler, and for some
files, including `binder.h`, they have custom code where they convert
the `enum`s into `#define`s and then call the common path on that. But
it does that only for `enum`s that match a regexp. So this particular
change does not work as-is, though would work if we give a suitable
name to the `enum` (I double-checked running the scripts with the
changes applied, and it seemed to work).

But who knows what other projects are doing anyway, so it may be best
to just hold on this change, since anyway Rust Binder is still an RFC
-- thus if Linus/Greg decide to merge the Rust support, this change
can still wait. And we may not need it anyway if we do it on the Rust
side.

In conclusion, I would drop this patch for the moment, and I will fix
it on our side.

(By the way, please note there was a v2:
https://lore.kernel.org/lkml/20220517102813.10310-1-ojeda@kernel.org/)

Cheers,
Miguel