Re: [PATCH] Staging:android: Silence some compiler warnings

[Zhengwang Ruan <ruan.zhengwang@gmail.com>]

-------- 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