2014-04-16 14:27:56

by Seunghun Lee

[permalink] [raw]
Subject: [PATCH] staging : android : uapi : fix coding style

This patch fix checkpatch.pl warnings and errors.

Signed-off-by: Seunghun Lee <[email protected]>
---
drivers/staging/android/uapi/binder.h | 2 +-
drivers/staging/android/uapi/ion.h | 20 +++++++++-----------
2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/android/uapi/binder.h b/drivers/staging/android/uapi/binder.h
index 904adb7..dba4cef 100644
--- a/drivers/staging/android/uapi/binder.h
+++ b/drivers/staging/android/uapi/binder.h
@@ -169,7 +169,7 @@ struct binder_ptr_cookie {
struct binder_handle_cookie {
__u32 handle;
binder_uintptr_t cookie;
-} __attribute__((packed));
+} __packed;

struct binder_pri_desc {
__s32 priority;
diff --git a/drivers/staging/android/uapi/ion.h b/drivers/staging/android/uapi/ion.h
index f09e7c1..36332dc 100644
--- a/drivers/staging/android/uapi/ion.h
+++ b/drivers/staging/android/uapi/ion.h
@@ -20,19 +20,17 @@
#include <linux/ioctl.h>
#include <linux/types.h>

-typedef int ion_user_handle_t;
-
/**
* enum ion_heap_types - list of all possible types of heaps
* @ION_HEAP_TYPE_SYSTEM: memory allocated via vmalloc
* @ION_HEAP_TYPE_SYSTEM_CONTIG: memory allocated via kmalloc
* @ION_HEAP_TYPE_CARVEOUT: memory allocated from a prereserved
- * carveout heap, allocations are physically
- * contiguous
+ * carveout heap, allocations are physically
+ * contiguous
* @ION_HEAP_TYPE_DMA: memory allocated via DMA API
* @ION_NUM_HEAPS: helper for iterating over heaps, a bit mask
- * is used to identify the heaps, so only 32
- * total heap types are supported
+ * is used to identify the heaps, so only 32
+ * total heap types are supported
*/
enum ion_heap_type {
ION_HEAP_TYPE_SYSTEM,
@@ -50,7 +48,7 @@ enum ion_heap_type {
#define ION_HEAP_CARVEOUT_MASK (1 << ION_HEAP_TYPE_CARVEOUT)
#define ION_HEAP_TYPE_DMA_MASK (1 << ION_HEAP_TYPE_DMA)

-#define ION_NUM_HEAP_IDS sizeof(unsigned int) * 8
+#define ION_NUM_HEAP_IDS (sizeof(unsigned int) * 8)

/**
* allocation flags - the lower 16 bits are used by core ion, the upper 16
@@ -78,7 +76,7 @@ enum ion_heap_type {
* @align: required alignment of the allocation
* @heap_id_mask: mask of heap ids to allocate from
* @flags: flags passed to heap
- * @handle: pointer that will be populated with a cookie to use to
+ * @handle: pointer that will be populated with a cookie to use to
* refer to this allocation
*
* Provided by userspace as an argument to the ioctl
@@ -88,7 +86,7 @@ struct ion_allocation_data {
size_t align;
unsigned int heap_id_mask;
unsigned int flags;
- ion_user_handle_t handle;
+ int handle;
};

/**
@@ -102,7 +100,7 @@ struct ion_allocation_data {
* provides the file descriptor and the kernel returns the handle.
*/
struct ion_fd_data {
- ion_user_handle_t handle;
+ int handle;
int fd;
};

@@ -111,7 +109,7 @@ struct ion_fd_data {
* @handle: a handle
*/
struct ion_handle_data {
- ion_user_handle_t handle;
+ int handle;
};

/**
--
1.7.9.5


2014-04-16 14:39:57

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] staging : android : uapi : fix coding style

(adding John Stultz)

On Wed, 2014-04-16 at 23:27 +0900, Seunghun Lee wrote:
> This patch fix checkpatch.pl warnings and errors.
[]
> diff --git a/drivers/staging/android/uapi/binder.h b/drivers/staging/android/uapi/binder.h
[]
> @@ -169,7 +169,7 @@ struct binder_ptr_cookie {
> struct binder_handle_cookie {
> __u32 handle;
> binder_uintptr_t cookie;
> -} __attribute__((packed));
> +} __packed;

If this .h file is meant to be a user-space #include,
then it should not use the kernel specific __packed
but keep the __attribute__((packed))

It does use __u32 though and that's generally
kernel specific.

John? Does any of these binder uapi files need a
bit more sorting out?

2014-04-16 14:44:22

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH] staging : android : uapi : fix coding style

Subject is not correct format. Should be:

[PATCH] staging: android: uapi: fix coding style

On Wed, Apr 16, 2014 at 11:27:39PM +0900, Seunghun Lee wrote:
> This patch fix checkpatch.pl warnings and errors.
>

Patch does too many things at once. Split it up.

> Signed-off-by: Seunghun Lee <[email protected]>
> ---
> drivers/staging/android/uapi/binder.h | 2 +-
> drivers/staging/android/uapi/ion.h | 20 +++++++++-----------
> 2 files changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/staging/android/uapi/binder.h b/drivers/staging/android/uapi/binder.h
> index 904adb7..dba4cef 100644
> --- a/drivers/staging/android/uapi/binder.h
> +++ b/drivers/staging/android/uapi/binder.h
> @@ -169,7 +169,7 @@ struct binder_ptr_cookie {
> struct binder_handle_cookie {
> __u32 handle;
> binder_uintptr_t cookie;
> -} __attribute__((packed));
> +} __packed;
>
> struct binder_pri_desc {
> __s32 priority;
> diff --git a/drivers/staging/android/uapi/ion.h b/drivers/staging/android/uapi/ion.h
> index f09e7c1..36332dc 100644
> --- a/drivers/staging/android/uapi/ion.h
> +++ b/drivers/staging/android/uapi/ion.h
> @@ -20,19 +20,17 @@
> #include <linux/ioctl.h>
> #include <linux/types.h>
>
> -typedef int ion_user_handle_t;

This typedef is probably a valid use. Plus it looks to be part of the
user space API. Just leave it alone.

regards,
dan carpenter

2014-04-16 15:27:45

by Seunghun Lee

[permalink] [raw]
Subject: Re: [PATCH] staging : android : uapi : fix coding style


On 04/16/2014 11:44 PM, Dan Carpenter wrote:
> Subject is not correct format. Should be:
>
> [PATCH] staging: android: uapi: fix coding style
>
> On Wed, Apr 16, 2014 at 11:27:39PM +0900, Seunghun Lee wrote:
>> This patch fix checkpatch.pl warnings and errors.
>>
> Patch does too many things at once. Split it up.
>
>> Signed-off-by: Seunghun Lee <[email protected]>
>> ---
>> drivers/staging/android/uapi/binder.h | 2 +-
>> drivers/staging/android/uapi/ion.h | 20 +++++++++-----------
>> 2 files changed, 10 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/staging/android/uapi/binder.h b/drivers/staging/android/uapi/binder.h
>> index 904adb7..dba4cef 100644
>> --- a/drivers/staging/android/uapi/binder.h
>> +++ b/drivers/staging/android/uapi/binder.h
>> @@ -169,7 +169,7 @@ struct binder_ptr_cookie {
>> struct binder_handle_cookie {
>> __u32 handle;
>> binder_uintptr_t cookie;
>> -} __attribute__((packed));
>> +} __packed;
>>
>> struct binder_pri_desc {
>> __s32 priority;
>> diff --git a/drivers/staging/android/uapi/ion.h b/drivers/staging/android/uapi/ion.h
>> index f09e7c1..36332dc 100644
>> --- a/drivers/staging/android/uapi/ion.h
>> +++ b/drivers/staging/android/uapi/ion.h
>> @@ -20,19 +20,17 @@
>> #include <linux/ioctl.h>
>> #include <linux/types.h>
>>
>> -typedef int ion_user_handle_t;
> This typedef is probably a valid use. Plus it looks to be part of the
> user space API. Just leave it alone.
>
> regards,
> dan carpenter
>
Thanks for review.
You're right, I sent the wrong patch.
I'll resend it.
Thanks.

2014-04-16 16:36:19

by John Stultz

[permalink] [raw]
Subject: Re: [PATCH] staging : android : uapi : fix coding style

On 04/16/2014 07:39 AM, Joe Perches wrote:
> On Wed, 2014-04-16 at 23:27 +0900, Seunghun Lee wrote:
>> This patch fix checkpatch.pl warnings and errors.
> []
>> diff --git a/drivers/staging/android/uapi/binder.h b/drivers/staging/android/uapi/binder.h
> []
>> @@ -169,7 +169,7 @@ struct binder_ptr_cookie {
>> struct binder_handle_cookie {
>> __u32 handle;
>> binder_uintptr_t cookie;
>> -} __attribute__((packed));
>> +} __packed;
> If this .h file is meant to be a user-space #include,
> then it should not use the kernel specific __packed
> but keep the __attribute__((packed))

Agreed.

> It does use __u32 though and that's generally
> kernel specific.

Hmm. Theres a ton of __u32 usage in include/uapi/* as well as typedefs
for it too.
include/uapi/asm-generic/int-l64.h:typedef unsigned int __u32;
include/uapi/asm-generic/int-ll64.h:typedef unsigned int __u32;

> John? Does any of these binder uapi files need a
> bit more sorting out?
I suspect this is ok, but Cc'ing Colin to give him a heads up, as it
would probably cause trouble w/ their libc headers first.


thanks
-john

2014-04-16 16:54:45

by Colin Cross

[permalink] [raw]
Subject: Re: [PATCH] staging : android : uapi : fix coding style

(resending without the html)

On Wed, Apr 16, 2014 at 9:36 AM, John Stultz <[email protected]> wrote:
>
> On 04/16/2014 07:39 AM, Joe Perches wrote:
> > On Wed, 2014-04-16 at 23:27 +0900, Seunghun Lee wrote:
> >> This patch fix checkpatch.pl warnings and errors.
> > []
> >> diff --git a/drivers/staging/android/uapi/binder.h b/drivers/staging/android/uapi/binder.h
> > []
> >> @@ -169,7 +169,7 @@ struct binder_ptr_cookie {
> >> struct binder_handle_cookie {
> >> __u32 handle;
> >> binder_uintptr_t cookie;
> >> -} __attribute__((packed));
> >> +} __packed;
> > If this .h file is meant to be a user-space #include,
> > then it should not use the kernel specific __packed
> > but keep the __attribute__((packed))
>
> Agreed.
>
> > It does use __u32 though and that's generally
> > kernel specific.
>
> Hmm. Theres a ton of __u32 usage in include/uapi/* as well as typedefs
> for it too.
> include/uapi/asm-generic/int-l64.h:typedef unsigned int __u32;
> include/uapi/asm-generic/int-ll64.h:typedef unsigned int __u32;
>
> > John? Does any of these binder uapi files need a
> > bit more sorting out?
> I suspect this is ok, but Cc'ing Colin to give him a heads up, as it
> would probably cause trouble w/ their libc headers first.

__u32 is explicitly for use in userspace headers so it doesn't
conflict with userspace definitions of u32, uint32_t, etc. It gets
set through #include <linux/types.h> when a copy of include/uapi is in
the include path.

>From Documentation/CodingStyle:
(e) Types safe for use in userspace.

In certain structures which are visible to userspace, we cannot
require C99 types and cannot use the 'u32' form above. Thus, we
use __u32 and similar types in all structures which are shared
with userspace.