* Re: [PATCH] Staging:android: Silence some compiler warnings
2012-02-24 11:52 [PATCH] Staging:android: Silence some compiler warnings Zhengwang Ruan
@ 2012-02-24 12:46 ` Dan Carpenter
2012-02-25 4:03 ` Zhengwang Ruan
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2012-02-24 12:46 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 2375 bytes --]
Not bad.
On Fri, Feb 24, 2012 at 07:52:00PM +0800, Zhengwang Ruan wrote:
> There are some compiler warnings, this is to silence them.
When there are compiler warnings, cut and paste the compiler
warnings into the message here.
>
> Signed-off-by: Zhengwang Ruan <ruan.zhengwang@gmail.com>
> ---
> drivers/staging/android/binder.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
> index f0b7e66..c4e616c 100644
> --- a/drivers/staging/android/binder.c
> +++ b/drivers/staging/android/binder.c
> @@ -104,7 +104,7 @@ static uint32_t binder_debug_mask = BINDER_DEBUG_USER_ERROR |
> module_param_named(debug_mask, binder_debug_mask, uint, S_IWUSR | S_IRUGO);
>
> static int binder_debug_no_lock;
> -module_param_named(proc_no_lock, binder_debug_no_lock, bool, S_IWUSR | S_IRUGO);
> +module_param_named(proc_no_lock, binder_debug_no_lock, int, S_IWUSR | S_IRUGO);
Instead of doing this, it would be better to change the line before
and declare binder_debug_no_lock as a bool.
>
> static DECLARE_WAIT_QUEUE_HEAD(binder_user_error_wait);
> static int binder_stop_on_user_error;
> @@ -716,8 +716,8 @@ static struct binder_buffer *binder_alloc_buf(struct binder_proc *proc,
> size_t offsets_size, int is_async)
> {
> struct rb_node *n = proc->free_buffers.rb_node;
> - struct binder_buffer *buffer;
> - size_t buffer_size;
> + struct binder_buffer *buffer = NULL;
> + size_t buffer_size = 0;
The other option would be to use the uninitialized_var() macro. On
fast paths we would probably insist on that, but for this probably
no one cares.
When you fix uninitialized variable warnings, please comment in the
changelog whether the variable can ever be used uninitialized in
real life. (Is GCC correct?).
This should probably be split into two patches. One for the type
change and one for the uninitialized variables. We could probably
merge it as is, but definitely no one will complain if you split it
apart.
Use the ./scripts/get_maintainer.pl to find where to resend this
patch. Every kernel patch has to be CC'd to a mailing list. I
wouldn't CC lkml for this. They don't care about trivial patches
to staging. Just devel@driverdev.osuosl.org is fine.
regards,
dan carpenter
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Staging:android: Silence some compiler warnings
2012-02-24 11:52 [PATCH] Staging:android: Silence some compiler warnings Zhengwang Ruan
2012-02-24 12:46 ` Dan Carpenter
@ 2012-02-25 4:03 ` Zhengwang Ruan
2012-02-25 7:32 ` Julia Lawall
2012-02-25 14:33 ` Dan Carpenter
3 siblings, 0 replies; 5+ messages in thread
From: Zhengwang Ruan @ 2012-02-25 4:03 UTC (permalink / raw)
To: kernel-janitors
-------- Original Message --------
From: Dan Carpenter
Sent: 2012年02月24日 星期五 15时46分55秒
To: Zhengwang Ruan
Subject: Re: [PATCH] Staging:android: Silence some compiler warnings
> Not bad.
>
> On Fri, Feb 24, 2012 at 07:52:00PM +0800, Zhengwang Ruan wrote:
>> There are some compiler warnings, this is to silence them.
> When there are compiler warnings, cut and paste the compiler
> warnings into the message here.
>
>> Signed-off-by: Zhengwang Ruan <ruan.zhengwang@gmail.com>
>> ---
>> drivers/staging/android/binder.c | 6 +++---
>> 1 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
>> index f0b7e66..c4e616c 100644
>> --- a/drivers/staging/android/binder.c
>> +++ b/drivers/staging/android/binder.c
>> @@ -104,7 +104,7 @@ static uint32_t binder_debug_mask = BINDER_DEBUG_USER_ERROR |
>> module_param_named(debug_mask, binder_debug_mask, uint, S_IWUSR | S_IRUGO);
>>
>> static int binder_debug_no_lock;
>> -module_param_named(proc_no_lock, binder_debug_no_lock, bool, S_IWUSR | S_IRUGO);
>> +module_param_named(proc_no_lock, binder_debug_no_lock, int, S_IWUSR | S_IRUGO);
> Instead of doing this, it would be better to change the line before
> and declare binder_debug_no_lock as a bool.
>
Actually, I did change the type for binder_debug_no_lock as a bool until
I noticed binder_debug_no_lock is to be assigned value to int variables
in the following context. I think this change may be helpful to keep
consistence with the following context and reduce confusion from a view
to read code.
>>
>> static DECLARE_WAIT_QUEUE_HEAD(binder_user_error_wait);
>> static int binder_stop_on_user_error;
>> @@ -716,8 +716,8 @@ static struct binder_buffer *binder_alloc_buf(struct binder_proc *proc,
>> size_t offsets_size, int is_async)
>> {
>> struct rb_node *n = proc->free_buffers.rb_node;
>> - struct binder_buffer *buffer;
>> - size_t buffer_size;
>> + struct binder_buffer *buffer = NULL;
>> + size_t buffer_size = 0;
> The other option would be to use the uninitialized_var() macro. On
> fast paths we would probably insist on that, but for this probably
> no one cares.
>
Good, I will change it with using uninitialized_var().
> When you fix uninitialized variable warnings, please comment in the
> changelog whether the variable can ever be used uninitialized in
> real life. (Is GCC correct?).
>
> This should probably be split into two patches. One for the type
> change and one for the uninitialized variables. We could probably
> merge it as is, but definitely no one will complain if you split it
> apart.
Ok, I will split this patch into two parts, and comment the changelog
respectively for each.
> Use the ./scripts/get_maintainer.pl to find where to resend this
Is it a common tool for community, how could I get it?
Thanks,
-Zhengwang
> patch. Every kernel patch has to be CC'd to a mailing list. I
> wouldn't CC lkml for this. They don't care about trivial patches
> to staging. Just devel@driverdev.osuosl.org is fine.
>
> regards,
> dan carpenter
>
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Staging:android: Silence some compiler warnings
2012-02-24 11:52 [PATCH] Staging:android: Silence some compiler warnings Zhengwang Ruan
2012-02-24 12:46 ` Dan Carpenter
2012-02-25 4:03 ` Zhengwang Ruan
@ 2012-02-25 7:32 ` Julia Lawall
2012-02-25 14:33 ` Dan Carpenter
3 siblings, 0 replies; 5+ messages in thread
From: Julia Lawall @ 2012-02-25 7:32 UTC (permalink / raw)
To: kernel-janitors
>> Use the ./scripts/get_maintainer.pl to find where to resend this
>
> Is it a common tool for community, how could I get it?
It is part of your Linux source tree.
julia
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Staging:android: Silence some compiler warnings
2012-02-24 11:52 [PATCH] Staging:android: Silence some compiler warnings Zhengwang Ruan
` (2 preceding siblings ...)
2012-02-25 7:32 ` Julia Lawall
@ 2012-02-25 14:33 ` Dan Carpenter
3 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2012-02-25 14:33 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1062 bytes --]
On Sat, Feb 25, 2012 at 12:03:56PM +0800, Zhengwang Ruan wrote:
> >> static int binder_debug_no_lock;
> >> -module_param_named(proc_no_lock, binder_debug_no_lock, bool, S_IWUSR | S_IRUGO);
> >> +module_param_named(proc_no_lock, binder_debug_no_lock, int, S_IWUSR | S_IRUGO);
> > Instead of doing this, it would be better to change the line before
> > and declare binder_debug_no_lock as a bool.
> >
>
> Actually, I did change the type for binder_debug_no_lock as a bool until
> I noticed binder_debug_no_lock is to be assigned value to int variables
> in the following context. I think this change may be helpful to keep
> consistence with the following context and reduce confusion from a view
> to read code.
>
The code isn't confusing... But anyway, the change affects how the
option is presented to the user, and it should be bool.
> > Use the ./scripts/get_maintainer.pl to find where to resend this
>
> Is it a common tool for community, how could I get it?
>
It's inside the kernel source tree.
regards,
dan carpenter
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread