diff mbox

jbd/jbd2: add pointer type conversion on void *arg from void * to journal_t * explicitly in kjournald/kjournald2

Message ID 4E38AB50.9040804@gmail.com
State New, archived
Headers show

Commit Message

Wang Sheng-Hui Aug. 3, 2011, 1:58 a.m. UTC
The patch is against 3.0

The arg of kjournald/kjournald2 is void *, and points to type
journal_t. We should convert it to journal_t * explicitly in
the kjournald/kjournald2 function body.

Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com>
---
 fs/jbd/journal.c  |    2 +-
 fs/jbd2/journal.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Li Zefan Aug. 3, 2011, 2:04 a.m. UTC | #1
09:58, Wang Sheng-Hui wrote:
> The patch is against 3.0
> 
> The arg of kjournald/kjournald2 is void *, and points to type
> journal_t. We should convert it to journal_t * explicitly in
> the kjournald/kjournald2 function body.
> 

Why?

Implicit convertion from void * to foo * is ok. Did the compiler
complain about this to you?

> Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com>
> ---
>  fs/jbd/journal.c  |    2 +-
>  fs/jbd2/journal.c |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/jbd/journal.c b/fs/jbd/journal.c
> index e2d4285..32956fd 100644
> --- a/fs/jbd/journal.c
> +++ b/fs/jbd/journal.c
> @@ -116,7 +116,7 @@ static void commit_timeout(unsigned long __data)
>  
>  static int kjournald(void *arg)
>  {
> -	journal_t *journal = arg;
> +	journal_t *journal = (journal_t *)arg;
>  	transaction_t *transaction;
>  
>  	/*
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 0dfa5b5..c4f4bfc 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -129,7 +129,7 @@ static void commit_timeout(unsigned long __data)
>  
>  static int kjournald2(void *arg)
>  {
> -	journal_t *journal = arg;
> +	journal_t *journal = (journal_t *)arg;
>  	transaction_t *transaction;
>  
>  	/*
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wang Sheng-Hui Aug. 3, 2011, 2:17 a.m. UTC | #2
On 2011年08月03日 10:04, Li Zefan wrote:
> 09:58, Wang Sheng-Hui wrote:
>> The patch is against 3.0
>>
>> The arg of kjournald/kjournald2 is void *, and points to type
>> journal_t. We should convert it to journal_t * explicitly in
>> the kjournald/kjournald2 function body.
>>
> 
> Why?
> 
> Implicit convertion from void * to foo * is ok. Did the compiler
> complain about this to you?

I remember any * can be assigned directly to void * in ANSI C, but
void * should be converted to specific point * type. Right?

And I checked the code of kswapd, in which explicitly conversion is
taken on the arg *. I think it should do so in journal.c too.

> 
>> Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com>
>> ---
>>  fs/jbd/journal.c  |    2 +-
>>  fs/jbd2/journal.c |    2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/jbd/journal.c b/fs/jbd/journal.c
>> index e2d4285..32956fd 100644
>> --- a/fs/jbd/journal.c
>> +++ b/fs/jbd/journal.c
>> @@ -116,7 +116,7 @@ static void commit_timeout(unsigned long __data)
>>  
>>  static int kjournald(void *arg)
>>  {
>> -	journal_t *journal = arg;
>> +	journal_t *journal = (journal_t *)arg;
>>  	transaction_t *transaction;
>>  
>>  	/*
>> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
>> index 0dfa5b5..c4f4bfc 100644
>> --- a/fs/jbd2/journal.c
>> +++ b/fs/jbd2/journal.c
>> @@ -129,7 +129,7 @@ static void commit_timeout(unsigned long __data)
>>  
>>  static int kjournald2(void *arg)
>>  {
>> -	journal_t *journal = arg;
>> +	journal_t *journal = (journal_t *)arg;
>>  	transaction_t *transaction;
>>  
>>  	/*

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Li Zefan Aug. 3, 2011, 2:31 a.m. UTC | #3
Wang Sheng-Hui wrote:
> On 2011年08月03日 10:04, Li Zefan wrote:
>> 09:58, Wang Sheng-Hui wrote:
>>> The patch is against 3.0
>>>
>>> The arg of kjournald/kjournald2 is void *, and points to type
>>> journal_t. We should convert it to journal_t * explicitly in
>>> the kjournald/kjournald2 function body.
>>>
>>
>> Why?
>>
>> Implicit convertion from void * to foo * is ok. Did the compiler
>> complain about this to you?
> 
> I remember any * can be assigned directly to void * in ANSI C, but
> void * should be converted to specific point * type. Right?
> 

No.

> And I checked the code of kswapd, in which explicitly conversion is
> taken on the arg *. I think it should do so in journal.c too.
> 

Please check more similar code.
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Cong Wang Aug. 3, 2011, 2:52 a.m. UTC | #4
On Wed, Aug 3, 2011 at 10:17 AM, Wang Sheng-Hui <shhuiw@gmail.com> wrote:
>
> I remember any * can be assigned directly to void * in ANSI C, but
> void * should be converted to specific point * type. Right?
>

C99 6.3.2.3

A pointer to void may be converted to or from a pointer to any
incomplete or object
type. A pointer to any incomplete or object type may be converted to a
pointer to void
and back again; the result shall compare equal to the original pointer.
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wang Sheng-Hui Aug. 3, 2011, 2:55 a.m. UTC | #5
On 2011年08月03日 10:52, Américo Wang wrote:
> On Wed, Aug 3, 2011 at 10:17 AM, Wang Sheng-Hui <shhuiw@gmail.com> wrote:
>>
>> I remember any * can be assigned directly to void * in ANSI C, but
>> void * should be converted to specific point * type. Right?
>>
> 
> C99 6.3.2.3

Got it. Thanks,

> 
> A pointer to void may be converted to or from a pointer to any
> incomplete or object
> type. A pointer to any incomplete or object type may be converted to a
> pointer to void
> and back again; the result shall compare equal to the original pointer.

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnaud Lacombe Aug. 3, 2011, 3:24 a.m. UTC | #6
Hi,

On Tue, Aug 2, 2011 at 10:17 PM, Wang Sheng-Hui <shhuiw@gmail.com> wrote:
> And I checked the code of kswapd, in which explicitly conversion is
> taken on the arg *. I think it should do so in journal.c too.
>
because it is done wrong somewhere is no argument to do it wrong here ...

 - Arnaud
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jesper Juhl Aug. 3, 2011, 5:38 a.m. UTC | #7
On Wed, 3 Aug 2011, Li Zefan wrote:

> 09:58, Wang Sheng-Hui wrote:
> > The patch is against 3.0
> > 
> > The arg of kjournald/kjournald2 is void *, and points to type
> > journal_t. We should convert it to journal_t * explicitly in
> > the kjournald/kjournald2 function body.
> > 
> 
> Why?
> 
> Implicit convertion from void * to foo * is ok. Did the compiler
> complain about this to you?
> 

Not only is it perfectly OK to rely on the implicit conversion, a lot of 
work has gone into actively removing such unneeded explicit casts over the 
years, so let's not introduce new ones.
diff mbox

Patch

diff --git a/fs/jbd/journal.c b/fs/jbd/journal.c
index e2d4285..32956fd 100644
--- a/fs/jbd/journal.c
+++ b/fs/jbd/journal.c
@@ -116,7 +116,7 @@  static void commit_timeout(unsigned long __data)
 
 static int kjournald(void *arg)
 {
-	journal_t *journal = arg;
+	journal_t *journal = (journal_t *)arg;
 	transaction_t *transaction;
 
 	/*
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 0dfa5b5..c4f4bfc 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -129,7 +129,7 @@  static void commit_timeout(unsigned long __data)
 
 static int kjournald2(void *arg)
 {
-	journal_t *journal = arg;
+	journal_t *journal = (journal_t *)arg;
 	transaction_t *transaction;
 
 	/*