binder: use enum for binder ioctls
Commit Message
All of the other constants in this file are defined using enums, so make
the constants more consistent by defining the ioctls in an enum as well.
This is necessary for Rust Binder since the _IO macros are too
complicated for bindgen to see that they expand to integer constants.
Replacing the #defines with an enum forces bindgen to evaluate them
properly, which allows us to access them from Rust.
I originally intended to include this change in the first patch of the
Rust Binder patchset [1], but at plumbers Carlos Llamas told me that
this change has been discussed previously [2] and suggested that I send
it upstream separately.
Link: https://lore.kernel.org/rust-for-linux/20231101-rust-binder-v1-1-08ba9197f637@google.com/ [1]
Link: https://lore.kernel.org/all/YoIK2l6xbQMPGZHy@kroah.com/ [2]
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
include/uapi/linux/android/binder.h | 30 +++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)
base-commit: 33cc938e65a98f1d29d0a18403dbbee050dcad9a
Comments
On Fri, Dec 08, 2023 at 03:28:01PM +0000, Alice Ryhl wrote:
> All of the other constants in this file are defined using enums, so make
> the constants more consistent by defining the ioctls in an enum as well.
>
> This is necessary for Rust Binder since the _IO macros are too
> complicated for bindgen to see that they expand to integer constants.
> Replacing the #defines with an enum forces bindgen to evaluate them
> properly, which allows us to access them from Rust.
>
> I originally intended to include this change in the first patch of the
> Rust Binder patchset [1], but at plumbers Carlos Llamas told me that
> this change has been discussed previously [2] and suggested that I send
> it upstream separately.
>
> Link: https://lore.kernel.org/rust-for-linux/20231101-rust-binder-v1-1-08ba9197f637@google.com/ [1]
> Link: https://lore.kernel.org/all/YoIK2l6xbQMPGZHy@kroah.com/ [2]
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
Acked-by: Carlos Llamas <cmllamas@google.com>
Thanks,
--
Carlos Llamas
On Fri, Dec 08, 2023 at 03:28:01PM +0000, Alice Ryhl wrote:
> All of the other constants in this file are defined using enums, so make
> the constants more consistent by defining the ioctls in an enum as well.
>
> This is necessary for Rust Binder since the _IO macros are too
> complicated for bindgen to see that they expand to integer constants.
> Replacing the #defines with an enum forces bindgen to evaluate them
> properly, which allows us to access them from Rust.
Does this mean that we will have to wrap every ioctl definition in an
enum just to get access to it in rust code?
What makes these defines so magical that bindgen can't decode them? Is
it just the complexity of the C preprocessor logic involved? Any plans
for bindgen to resolve this?
Note, I'm not objecting to this patch (I'll queue it up next week when I
get the chance), just curious about the rust tooling side here.
thanks,
greg k-h
> Does this mean that we will have to wrap every ioctl definition in an
> enum just to get access to it in rust code?
Currently, yes (or one can build them on the Rust side using the `_IO*` const
functions that are in mainline at `rust/kernel/ioctl.rs`).
> What makes these defines so magical that bindgen can't decode them? Is
> it just the complexity of the C preprocessor logic involved? Any plans
> for bindgen to resolve this?
Yeah, currently bindgen only resolves "trivial" object-like macros. As soon as
a macro is more complex it does not work, even if it would still resolve into
a constant. The upstream issue for this particular case (a macro that uses
other function-like macros) uses `_IO*`s as the example, in fact, and is at:
https://github.com/rust-lang/rust-bindgen/issues/753
which we track in our bindgen list at:
https://github.com/Rust-for-Linux/linux/issues/353
There are several ways forward for bindgen here. Ideally, libclang would give
the resolved value to bindgen. This may require changes in upstream Clang.
I contacted Aaron Ballman (the Clang maintainer, Cc'd) a while ago and
he kindly offered to review the changes if they were eventually needed.
Otherwise (or meanwhile), it is also possible to implement a workaround that
stores the information in a way that can be retrieved by bindgen by using
the macro in some way (e.g. initializing a variable and asking for the value
of the variable). It is ugly, but it should work (at least for many cases --
there may be other compounding issues with e.g. 128-bit integers).
John Baublitz (Cc'd) has spent some time on this and, from what I can tell from
the issue, we may be waiting on an answer from bindgen (Cc'ing Emilio and
Christian, the bindgen maintainers) on whether the workaround is OK for them.
The workaround would be nice to have even if we change upstream Clang, because
it would help in many cases we care about, without having to wait until we get
a new LLVM released and so on.
Hope that helps!
Cheers,
Miguel
@@ -251,20 +251,22 @@ struct binder_extended_error {
__s32 param;
};
-#define BINDER_WRITE_READ _IOWR('b', 1, struct binder_write_read)
-#define BINDER_SET_IDLE_TIMEOUT _IOW('b', 3, __s64)
-#define BINDER_SET_MAX_THREADS _IOW('b', 5, __u32)
-#define BINDER_SET_IDLE_PRIORITY _IOW('b', 6, __s32)
-#define BINDER_SET_CONTEXT_MGR _IOW('b', 7, __s32)
-#define BINDER_THREAD_EXIT _IOW('b', 8, __s32)
-#define BINDER_VERSION _IOWR('b', 9, struct binder_version)
-#define BINDER_GET_NODE_DEBUG_INFO _IOWR('b', 11, struct binder_node_debug_info)
-#define BINDER_GET_NODE_INFO_FOR_REF _IOWR('b', 12, struct binder_node_info_for_ref)
-#define BINDER_SET_CONTEXT_MGR_EXT _IOW('b', 13, struct flat_binder_object)
-#define BINDER_FREEZE _IOW('b', 14, struct binder_freeze_info)
-#define BINDER_GET_FROZEN_INFO _IOWR('b', 15, struct binder_frozen_status_info)
-#define BINDER_ENABLE_ONEWAY_SPAM_DETECTION _IOW('b', 16, __u32)
-#define BINDER_GET_EXTENDED_ERROR _IOWR('b', 17, struct binder_extended_error)
+enum {
+ BINDER_WRITE_READ = _IOWR('b', 1, struct binder_write_read),
+ BINDER_SET_IDLE_TIMEOUT = _IOW('b', 3, __s64),
+ BINDER_SET_MAX_THREADS = _IOW('b', 5, __u32),
+ BINDER_SET_IDLE_PRIORITY = _IOW('b', 6, __s32),
+ BINDER_SET_CONTEXT_MGR = _IOW('b', 7, __s32),
+ BINDER_THREAD_EXIT = _IOW('b', 8, __s32),
+ BINDER_VERSION = _IOWR('b', 9, struct binder_version),
+ BINDER_GET_NODE_DEBUG_INFO = _IOWR('b', 11, struct binder_node_debug_info),
+ BINDER_GET_NODE_INFO_FOR_REF = _IOWR('b', 12, struct binder_node_info_for_ref),
+ BINDER_SET_CONTEXT_MGR_EXT = _IOW('b', 13, struct flat_binder_object),
+ BINDER_FREEZE = _IOW('b', 14, struct binder_freeze_info),
+ BINDER_GET_FROZEN_INFO = _IOWR('b', 15, struct binder_frozen_status_info),
+ BINDER_ENABLE_ONEWAY_SPAM_DETECTION = _IOW('b', 16, __u32),
+ BINDER_GET_EXTENDED_ERROR = _IOWR('b', 17, struct binder_extended_error),
+};
/*
* NOTE: Two special error codes you should check for when calling