[PATCH 1/4] x86: remove cast from void*

[PATCH 1/4] x86: remove cast from void*

[matt mooney]

Unnecessary cast from void* in assignment.

Signed-off-by: matt mooney <mfm@muteddisk.com>
---
 arch/x86/xen/mmu.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
index 42086ac..7436283 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -737,7 +737,7 @@ pgd_t *xen_get_user_pgd(pgd_t *pgd)
 
 	if (offset < pgd_index(USER_LIMIT)) {
 		struct page *page = virt_to_page(pgd_page);
-		user_ptr = (pgd_t *)page->private;
+		user_ptr = page->private;
 		if (user_ptr)
 			user_ptr += offset;
 	}
-- 
1.7.2.1

[PATCH 1/4] x86: remove cast from void*

[matt mooney]

Unnecessary cast from void* in assignment.

Signed-off-by: matt mooney <mfm@muteddisk.com>
---
 arch/x86/xen/mmu.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
index 42086ac..7436283 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -737,7 +737,7 @@ pgd_t *xen_get_user_pgd(pgd_t *pgd)
 
 	if (offset < pgd_index(USER_LIMIT)) {
 		struct page *page = virt_to_page(pgd_page);
-		user_ptr = (pgd_t *)page->private;
+		user_ptr = page->private;
 		if (user_ptr)
 			user_ptr += offset;
 	}
-- 
1.7.2.1

Re: [PATCH 1/4] x86: remove cast from void*

[Jeremy Fitzhardinge]

 On 09/13/2010 10:18 PM, matt mooney wrote:
> Unnecessary cast from void* in assignment.

Not very keen on this.  The cast may not be strictly required, but it
does document what's going on there.

    J

> Signed-off-by: matt mooney <mfm@muteddisk.com>
> ---
>  arch/x86/xen/mmu.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
> index 42086ac..7436283 100644
> --- a/arch/x86/xen/mmu.c
> +++ b/arch/x86/xen/mmu.c
> @@ -737,7 +737,7 @@ pgd_t *xen_get_user_pgd(pgd_t *pgd)
>  
>  	if (offset < pgd_index(USER_LIMIT)) {
>  		struct page *page = virt_to_page(pgd_page);
> -		user_ptr = (pgd_t *)page->private;
> +		user_ptr = page->private;
>  		if (user_ptr)
>  			user_ptr += offset;
>  	}

Re: [PATCH 1/4] x86: remove cast from void*

[Jeremy Fitzhardinge]

 On 09/13/2010 10:18 PM, matt mooney wrote:
> Unnecessary cast from void* in assignment.

Not very keen on this.  The cast may not be strictly required, but it
does document what's going on there.

    J

> Signed-off-by: matt mooney <mfm@muteddisk.com>
> ---
>  arch/x86/xen/mmu.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
> index 42086ac..7436283 100644
> --- a/arch/x86/xen/mmu.c
> +++ b/arch/x86/xen/mmu.c
> @@ -737,7 +737,7 @@ pgd_t *xen_get_user_pgd(pgd_t *pgd)
>  
>  	if (offset < pgd_index(USER_LIMIT)) {
>  		struct page *page = virt_to_page(pgd_page);
> -		user_ptr = (pgd_t *)page->private;
> +		user_ptr = page->private;
>  		if (user_ptr)
>  			user_ptr += offset;
>  	}

Re: [PATCH 1/4] x86: remove cast from void*

[H. Peter Anvin]

On 09/14/2010 10:49 AM, Jeremy Fitzhardinge wrote:
>  On 09/13/2010 10:18 PM, matt mooney wrote:
>> Unnecessary cast from void* in assignment.
> 
> Not very keen on this.  The cast may not be strictly required, but it
> does document what's going on there.
> 

But unnecessary casts are problematic in that if the type changes, they
can hide a real bug in the future.
	
	-hpa

Re: [PATCH 1/4] x86: remove cast from void*

[H. Peter Anvin]

On 09/14/2010 10:49 AM, Jeremy Fitzhardinge wrote:
>  On 09/13/2010 10:18 PM, matt mooney wrote:
>> Unnecessary cast from void* in assignment.
> 
> Not very keen on this.  The cast may not be strictly required, but it
> does document what's going on there.
> 

But unnecessary casts are problematic in that if the type changes, they
can hide a real bug in the future.
	
	-hpa

Re: [PATCH 1/4] x86: remove cast from void*

[Jeremy Fitzhardinge]

 On 09/14/2010 11:20 AM, H. Peter Anvin wrote:
> On 09/14/2010 10:49 AM, Jeremy Fitzhardinge wrote:
>>  On 09/13/2010 10:18 PM, matt mooney wrote:
>>> Unnecessary cast from void* in assignment.
>> Not very keen on this.  The cast may not be strictly required, but it
>> does document what's going on there.
>>
> But unnecessary casts are problematic in that if the type changes, they
> can hide a real bug in the future.

If page->private changes in a way which invalidates this kind of usage,
I would hope it would also change name to something more descriptive of
its new role.  If it changes type but is broadly compatible with this
usage (say, void * -> unsigned long), then the cast won't matter, or
will become a requirement.

I'm more concerned about "user_ptr" changing type, in which case the
cast will highlight the problem.

In general, the casts from these kinds of generic/polymorphic fields let
you easily eyeball how they're being used, without having to work out
what the type of the other side is.

    J

Re: [PATCH 1/4] x86: remove cast from void*

[Jeremy Fitzhardinge]

 On 09/14/2010 11:20 AM, H. Peter Anvin wrote:
> On 09/14/2010 10:49 AM, Jeremy Fitzhardinge wrote:
>>  On 09/13/2010 10:18 PM, matt mooney wrote:
>>> Unnecessary cast from void* in assignment.
>> Not very keen on this.  The cast may not be strictly required, but it
>> does document what's going on there.
>>
> But unnecessary casts are problematic in that if the type changes, they
> can hide a real bug in the future.

If page->private changes in a way which invalidates this kind of usage,
I would hope it would also change name to something more descriptive of
its new role.  If it changes type but is broadly compatible with this
usage (say, void * -> unsigned long), then the cast won't matter, or
will become a requirement.

I'm more concerned about "user_ptr" changing type, in which case the
cast will highlight the problem.

In general, the casts from these kinds of generic/polymorphic fields let
you easily eyeball how they're being used, without having to work out
what the type of the other side is.

    J

Re: [PATCH 1/4] x86: remove cast from void*

[Jeremy Fitzhardinge]

 On 09/13/2010 10:18 PM, matt mooney wrote:
> Unnecessary cast from void* in assignment.
>
> Signed-off-by: matt mooney <mfm@muteddisk.com>
> ---
>  arch/x86/xen/mmu.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
> index 42086ac..7436283 100644
> --- a/arch/x86/xen/mmu.c
> +++ b/arch/x86/xen/mmu.c
> @@ -737,7 +737,7 @@ pgd_t *xen_get_user_pgd(pgd_t *pgd)
>  
>  	if (offset < pgd_index(USER_LIMIT)) {
>  		struct page *page = virt_to_page(pgd_page);
> -		user_ptr = (pgd_t *)page->private;
> +		user_ptr = page->private;

Um, page->private is unsigned long anyway, so this is needed either way.

    J

Re: [PATCH 1/4] x86: remove cast from void*

[Jeremy Fitzhardinge]

 On 09/13/2010 10:18 PM, matt mooney wrote:
> Unnecessary cast from void* in assignment.
>
> Signed-off-by: matt mooney <mfm@muteddisk.com>
> ---
>  arch/x86/xen/mmu.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
> index 42086ac..7436283 100644
> --- a/arch/x86/xen/mmu.c
> +++ b/arch/x86/xen/mmu.c
> @@ -737,7 +737,7 @@ pgd_t *xen_get_user_pgd(pgd_t *pgd)
>  
>  	if (offset < pgd_index(USER_LIMIT)) {
>  		struct page *page = virt_to_page(pgd_page);
> -		user_ptr = (pgd_t *)page->private;
> +		user_ptr = page->private;

Um, page->private is unsigned long anyway, so this is needed either way.

    J

Re: [PATCH 1/4] x86: remove cast from void*

[matt mooney]

On 11:39 Tue 21 Sep     , Jeremy Fitzhardinge wrote:
>  On 09/13/2010 10:18 PM, matt mooney wrote:
> > Unnecessary cast from void* in assignment.
> >
> > Signed-off-by: matt mooney <mfm@muteddisk.com>
> > ---
> >  arch/x86/xen/mmu.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
> > index 42086ac..7436283 100644
> > --- a/arch/x86/xen/mmu.c
> > +++ b/arch/x86/xen/mmu.c
> > @@ -737,7 +737,7 @@ pgd_t *xen_get_user_pgd(pgd_t *pgd)
> >  
> >  	if (offset < pgd_index(USER_LIMIT)) {
> >  		struct page *page = virt_to_page(pgd_page);
> > -		user_ptr = (pgd_t *)page->private;
> > +		user_ptr = page->private;
> 
> Um, page->private is unsigned long anyway, so this is needed either way.
> 

You're right. I missed that one, sorry; I really thought I had verified it.

-mfm

Re: [PATCH 1/4] x86: remove cast from void*

[matt mooney]

On 11:39 Tue 21 Sep     , Jeremy Fitzhardinge wrote:
>  On 09/13/2010 10:18 PM, matt mooney wrote:
> > Unnecessary cast from void* in assignment.
> >
> > Signed-off-by: matt mooney <mfm@muteddisk.com>
> > ---
> >  arch/x86/xen/mmu.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
> > index 42086ac..7436283 100644
> > --- a/arch/x86/xen/mmu.c
> > +++ b/arch/x86/xen/mmu.c
> > @@ -737,7 +737,7 @@ pgd_t *xen_get_user_pgd(pgd_t *pgd)
> >  
> >  	if (offset < pgd_index(USER_LIMIT)) {
> >  		struct page *page = virt_to_page(pgd_page);
> > -		user_ptr = (pgd_t *)page->private;
> > +		user_ptr = page->private;
> 
> Um, page->private is unsigned long anyway, so this is needed either way.
> 

You're right. I missed that one, sorry; I really thought I had verified it.

-mfm