linux-next: manual merge of the rust tree with the char-misc tree

Stephen Rothwell posted 1 patch 3 years, 11 months ago
There is a newer version of this series
linux-next: manual merge of the rust tree with the char-misc tree
Posted by Stephen Rothwell 3 years, 11 months ago
Hi all,

Today's linux-next merge of the rust tree got a conflict in:

  include/uapi/linux/android/binder.h

between commit:

  bd32889e841c ("binder: add BINDER_GET_EXTENDED_ERROR ioctl")

from the char-misc tree and commit:

  d637f9ec1954 ("[RFC] drivers: android: Binder IPC in Rust")

from the rust tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc include/uapi/linux/android/binder.h
index e6ee8cae303b,a982c30dbbf5..000000000000
--- a/include/uapi/linux/android/binder.h
+++ b/include/uapi/linux/android/binder.h
@@@ -236,35 -236,21 +236,37 @@@ struct binder_frozen_status_info 
  	__u32            async_recv;
  };
  
 +/* struct binder_extened_error - extended error information
 + * @id:		identifier for the failed operation
 + * @command:	command as defined by binder_driver_return_protocol
 + * @param:	parameter holding a negative errno value
 + *
 + * Used with BINDER_GET_EXTENDED_ERROR. This extends the error information
 + * returned by the driver upon a failed operation. Userspace can pull this
 + * data to properly handle specific error scenarios.
 + */
 +struct binder_extended_error {
 +	__u32	id;
 +	__u32	command;
 +	__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
Re: linux-next: manual merge of the rust tree with the char-misc tree
Posted by Greg KH 3 years, 11 months ago
On Mon, May 16, 2022 at 05:13:29PM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the rust tree got a conflict in:
> 
>   include/uapi/linux/android/binder.h
> 
> between commit:
> 
>   bd32889e841c ("binder: add BINDER_GET_EXTENDED_ERROR ioctl")
> 
> from the char-misc tree and commit:
> 
>   d637f9ec1954 ("[RFC] drivers: android: Binder IPC in Rust")
> 
> from the rust tree.
> 
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging.  You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
> 
> -- 
> Cheers,
> Stephen Rothwell
> 
> diff --cc include/uapi/linux/android/binder.h
> index e6ee8cae303b,a982c30dbbf5..000000000000
> --- a/include/uapi/linux/android/binder.h
> +++ b/include/uapi/linux/android/binder.h
> @@@ -236,35 -236,21 +236,37 @@@ struct binder_frozen_status_info 
>   	__u32            async_recv;
>   };
>   
>  +/* struct binder_extened_error - extended error information
>  + * @id:		identifier for the failed operation
>  + * @command:	command as defined by binder_driver_return_protocol
>  + * @param:	parameter holding a negative errno value
>  + *
>  + * Used with BINDER_GET_EXTENDED_ERROR. This extends the error information
>  + * returned by the driver upon a failed operation. Userspace can pull this
>  + * data to properly handle specific error scenarios.
>  + */
>  +struct binder_extended_error {
>  +	__u32	id;
>  +	__u32	command;
>  +	__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),
> + };
>   

Odd, why does the Rust binder implementation require the IOCTL
definitions to be in an enum instead of a #define?

If that's required, I can take that type of change now to prevent merge
issues in the future.

thanks,

greg k-h
Re: linux-next: manual merge of the rust tree with the char-misc tree
Posted by Miguel Ojeda 3 years, 11 months ago
Hi Greg,

On Mon, May 16, 2022 at 9:49 AM Greg KH <greg@kroah.com> wrote:
>
> Odd, why does the Rust binder implementation require the IOCTL
> definitions to be in an enum instead of a #define?

It is because bindgen (the tool which generates the "raw" C bindings)
only works with simple C `#define`s. Thus there are two options when
one hits something like this: either copy-paste them manually into the
Rust side (which adds maintenance), or change them into real
objects/identifiers in the C side.

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.

Stephen: the resolution looks fine, thanks!

> If that's required, I can take that type of change now to prevent merge
> issues in the future.

We have two so far: this one (for the RFC Android patch) and another
small one in include/linux/spinlock.h (for the `sync` module).

So far I have put those changes in their respective patches, but if
you prefer I can send them independently too (like the prerequisite
patches).

Cheers,
Miguel
Re: linux-next: manual merge of the rust tree with the char-misc tree
Posted by Greg KH 3 years, 11 months ago
On Mon, May 16, 2022 at 10:18:45AM +0200, Miguel Ojeda wrote:
> Hi Greg,
> 
> On Mon, May 16, 2022 at 9:49 AM Greg KH <greg@kroah.com> wrote:
> >
> > Odd, why does the Rust binder implementation require the IOCTL
> > definitions to be in an enum instead of a #define?
> 
> It is because bindgen (the tool which generates the "raw" C bindings)
> only works with simple C `#define`s. Thus there are two options when
> one hits something like this: either copy-paste them manually into the
> Rust side (which adds maintenance), or change them into real
> objects/identifiers in the C side.
> 
> 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.
> 
> Stephen: the resolution looks fine, thanks!
> 
> > If that's required, I can take that type of change now to prevent merge
> > issues in the future.
> 
> We have two so far: this one (for the RFC Android patch) and another
> small one in include/linux/spinlock.h (for the `sync` module).
> 
> So far I have put those changes in their respective patches, but if
> you prefer I can send them independently too (like the prerequisite
> patches).

The binder one I can take now if you want to send that to me, as that
would make your development life easier.

thanks,

greg k-h