bindgen (a tool which generates the "raw" C bindings for Rust) only
works (so far) with "simple" C `#define`s. In order to avoid having
to manually maintain these constants in the (potential) Rust side,
this patch converts them into an `enum`.
There may be support in the future for expanding macros that end up in
a "numeric" one: https://github.com/rust-lang/rust-bindgen/issues/753.
Co-developed-by: Wedson Almeida Filho <[email protected]>
Signed-off-by: Wedson Almeida Filho <[email protected]>
Signed-off-by: Miguel Ojeda <[email protected]>
---
Two notes:
- Let me know if you prefer that I base this on top of a binder branch.
- Wedson should OK the changes for his `Signed-off-by` tag (and probably
he should be the author of the patch).
include/uapi/linux/android/binder.h | 28 +++++++++++++++-------------
1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/include/uapi/linux/android/binder.h b/include/uapi/linux/android/binder.h
index 11157fae8a8e..ed4d11a0bb99 100644
--- a/include/uapi/linux/android/binder.h
+++ b/include/uapi/linux/android/binder.h
@@ -236,19 +236,21 @@ struct binder_frozen_status_info {
__u32 async_recv;
};
-#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)
+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),
+};
/*
* NOTE: Two special error codes you should check for when calling
base-commit: 42226c989789d8da4af1de0c31070c96726d990c
--
2.36.1
On Mon, May 16, 2022 at 6:56 PM Greg Kroah-Hartman
<[email protected]> wrote:
>
> Yes, this does not apply to my char-misc.git tree in the char-misc-next
> branch on git.kernel.org as I think we have added some new binder ioctls
> recently.
Yeah, exactly, I thought you might want the other base to avoid the
conflicts -- sending it in a bit!
Cheers,
Miguel
On Mon, May 16, 2022 at 12:04:01PM +0200, Miguel Ojeda wrote:
> bindgen (a tool which generates the "raw" C bindings for Rust) only
> works (so far) with "simple" C `#define`s. In order to avoid having
> to manually maintain these constants in the (potential) Rust side,
> this patch converts them into an `enum`.
>
> There may be support in the future for expanding macros that end up in
> a "numeric" one: https://github.com/rust-lang/rust-bindgen/issues/753.
>
> Co-developed-by: Wedson Almeida Filho <[email protected]>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
> Signed-off-by: Miguel Ojeda <[email protected]>
> ---
> Two notes:
> - Let me know if you prefer that I base this on top of a binder branch.
Yes, this does not apply to my char-misc.git tree in the char-misc-next
branch on git.kernel.org as I think we have added some new binder ioctls
recently.
Or you can make it against linux-next with the rust stuff removed, that
also would work as well. But as-is, this patch does not work.
thanks,
greg k-h
On Mon, May 16, 2022 at 11:04 AM Miguel Ojeda <[email protected]> wrote:
>
> -#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)
> +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),
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.
Arnd
On Wed, May 18, 2022 at 9:52 AM Arnd Bergmann <[email protected]> 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/[email protected]/)
Cheers,
Miguel