Re: [PATCH v1 3/9] staging: android: binder: Add cmd == CMD_NAME handling

[Serban Constantinescu <Serban.Constantinescu@arm.com>]

On 05/12/13 08:40, Dan Carpenter wrote:
> On Wed, Dec 04, 2013 at 06:09:35PM +0000, Serban Constantinescu wrote:
>> This patch modifies the functions that need to be passed the explicit
>> command to use a boolean flag. This way we can reuse the code for 64bit
>> compat commands.
>>
>
> I don't understand this at all.  cmd seems like it should be 32 bits
> on both arches.
Command is 32bit for both arches but does not have the same value. Here 
is what happens, this patch make more sense when looking at the compat 
layer.

The binder commands differ between 32bit userspace and 64bit kernels. E.g:

> BR_INCREFS = _IOR('r', 7, struct binder_ptr_cookie)

> COMPAT_BR_INCREFS = _IOR('r', 7, struct compat_binder_ptr_cookie)

> struct compat_binder_ptr_cookie {
>     compat_uptr_t ptr;
>     compat_uptr_t cookie;
> };

> struct binder_ptr_cookie {
>     void *ptr;
>     void *cookie;
> };

(the IOR() macro encodes the size of the transaction - struct 
binder_ptr_cookie).

This enables me to use the same handler for:

* native 64bit kernel/ 64bit userspace

>           case BC_INCREFS_DONE:
>         case BC_ACQUIRE_DONE: {
>                  void __user *node_ptr;
>                  void *cookie;
>
>                  if (get_user(node_ptr, (void * __user *)ptr))
>                        return -EFAULT;
>                  ptr += sizeof(void *);
>                  if (get_user(cookie, (void * __user *)ptr))
>                        return -EFAULT;
>                  ptr += sizeof(void *);
>                  bc_increfs_done(proc, thread, cmd == BC_ACQUIRE_DONE, node_ptr, cookie);
>                  break;
>           }

* compat 64bit kernel/ 32bit userspace.

> +       case COMPAT_BC_INCREFS_DONE:
> +       case COMPAT_BC_ACQUIRE_DONE: {
> +               compat_uptr_t node_ptr;
> +               compat_uptr_t cookie;
> +
> +               if (get_user(node_ptr, (compat_uptr_t __user *)*ptr))
> +                       return -EFAULT;
> +               *ptr += sizeof(compat_uptr_t);
> +               if (get_user(cookie, (compat_uptr_t __user *)*ptr))
> +                       return -EFAULT;
> +               *ptr += sizeof(compat_uptr_t);
> +               bc_increfs_done(proc, thread, cmd == COMPAT_BC_ACQUIRE_DONE,
> +                       compat_ptr(node_ptr), compat_ptr(cookie));
> +               break;

where the prototype for bc_increfs_done() has changed to:

>  static void bc_increfs_done(struct binder_proc *proc,
> -        struct binder_thread *thread, uint32_t cmd,
> +        struct binder_thread *thread, bool acquire,

Obsucre on its own, I should add more details in the commit message.

Thanks,
Serban C