All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH -tip] x86: kvm/paging_tmpl.h intialize the variable before using it
@ 2009-05-20  4:29 Jaswinder Singh Rajput
  2009-05-24 12:21 ` Avi Kivity
  0 siblings, 1 reply; 8+ messages in thread
From: Jaswinder Singh Rajput @ 2009-05-20  4:29 UTC (permalink / raw
  To: Ingo Molnar, Avi Kivity, x86 maintainers, LKML


May be in some cases paging64_fetch() and paging32_fetch() will return sptep
without initialization.

Also fixes compilation warning:
  CC      arch/x86/kernel/io_delay.o
 arch/x86/kvm/paging_tmpl.h: In function ‘paging64_fetch’:
 arch/x86/kvm/paging_tmpl.h:279: warning: ‘sptep’ may be used uninitialized in this function
 arch/x86/kvm/paging_tmpl.h: In function ‘paging32_fetch’:
 arch/x86/kvm/paging_tmpl.h:279: warning: ‘sptep’ may be used uninitialized in this function

Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
---
 arch/x86/kvm/paging_tmpl.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
index 6bd7020..99cb10d 100644
--- a/arch/x86/kvm/paging_tmpl.h
+++ b/arch/x86/kvm/paging_tmpl.h
@@ -276,7 +276,7 @@ static u64 *FNAME(fetch)(struct kvm_vcpu *vcpu, gva_t addr,
 {
 	unsigned access = gw->pt_access;
 	struct kvm_mmu_page *shadow_page;
-	u64 spte, *sptep;
+	u64 spte, *sptep = NULL;
 	int direct;
 	gfn_t table_gfn;
 	int r;
-- 
1.6.1.1




^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH -tip] x86: kvm/paging_tmpl.h intialize the variable before using it
  2009-05-20  4:29 [PATCH -tip] x86: kvm/paging_tmpl.h intialize the variable before using it Jaswinder Singh Rajput
@ 2009-05-24 12:21 ` Avi Kivity
  2009-05-25  5:17   ` Jaswinder Singh Rajput
  2009-05-25  6:05   ` Ingo Molnar
  0 siblings, 2 replies; 8+ messages in thread
From: Avi Kivity @ 2009-05-24 12:21 UTC (permalink / raw
  To: Jaswinder Singh Rajput; +Cc: Ingo Molnar, x86 maintainers, LKML

Jaswinder Singh Rajput wrote:
> May be in some cases paging64_fetch() and paging32_fetch() will return sptep
> without initialization.
>
> Also fixes compilation warning:
>   CC      arch/x86/kernel/io_delay.o
>  arch/x86/kvm/paging_tmpl.h: In function ‘paging64_fetch’:
>  arch/x86/kvm/paging_tmpl.h:279: warning: ‘sptep’ may be used uninitialized in this function
>  arch/x86/kvm/paging_tmpl.h: In function ‘paging32_fetch’:
>  arch/x86/kvm/paging_tmpl.h:279: warning: ‘sptep’ may be used uninitialized in this function
>
> Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
> ---
>  arch/x86/kvm/paging_tmpl.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
> index 6bd7020..99cb10d 100644
> --- a/arch/x86/kvm/paging_tmpl.h
> +++ b/arch/x86/kvm/paging_tmpl.h
> @@ -276,7 +276,7 @@ static u64 *FNAME(fetch)(struct kvm_vcpu *vcpu, gva_t addr,
>  {
>  	unsigned access = gw->pt_access;
>  	struct kvm_mmu_page *shadow_page;
> -	u64 spte, *sptep;
> +	u64 spte, *sptep = NULL;
>  	int direct;
>  	gfn_t table_gfn;
>  	int r;
>   

It's a false alarm.  Isn't there a macro to shut up the warning?


-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH -tip] x86: kvm/paging_tmpl.h intialize the variable before using it
  2009-05-24 12:21 ` Avi Kivity
@ 2009-05-25  5:17   ` Jaswinder Singh Rajput
  2009-05-25  5:21     ` Avi Kivity
  2009-05-25  6:05   ` Ingo Molnar
  1 sibling, 1 reply; 8+ messages in thread
From: Jaswinder Singh Rajput @ 2009-05-25  5:17 UTC (permalink / raw
  To: Avi Kivity; +Cc: Ingo Molnar, x86 maintainers, LKML

Hello Avi,

On Sun, 2009-05-24 at 15:21 +0300, Avi Kivity wrote:
> Jaswinder Singh Rajput wrote:
> > May be in some cases paging64_fetch() and paging32_fetch() will return sptep
> > without initialization.
> >
> > Also fixes compilation warning:
> >   CC      arch/x86/kernel/io_delay.o
> >  arch/x86/kvm/paging_tmpl.h: In function ‘paging64_fetch’:
> >  arch/x86/kvm/paging_tmpl.h:279: warning: ‘sptep’ may be used uninitialized in this function
> >  arch/x86/kvm/paging_tmpl.h: In function ‘paging32_fetch’:
> >  arch/x86/kvm/paging_tmpl.h:279: warning: ‘sptep’ may be used uninitialized in this function
> >
> > Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
> > ---
> >  arch/x86/kvm/paging_tmpl.h |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
> > index 6bd7020..99cb10d 100644
> > --- a/arch/x86/kvm/paging_tmpl.h
> > +++ b/arch/x86/kvm/paging_tmpl.h
> > @@ -276,7 +276,7 @@ static u64 *FNAME(fetch)(struct kvm_vcpu *vcpu, gva_t addr,
> >  {
> >  	unsigned access = gw->pt_access;
> >  	struct kvm_mmu_page *shadow_page;
> > -	u64 spte, *sptep;
> > +	u64 spte, *sptep = NULL;
> >  	int direct;
> >  	gfn_t table_gfn;
> >  	int r;
> >   
> 
> It's a false alarm.  Isn't there a macro to shut up the warning?
> 

In arch/x86/kvm/paging_tmpl.h sptep is initialize only in :

for_each_shadow_entry(vcpu, addr, iterator) {

If we skip this, then we end up with wild sptep and returning it.

Do you still think it is a false alarm ?

--
JSR



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH -tip] x86: kvm/paging_tmpl.h intialize the variable before using it
  2009-05-25  5:17   ` Jaswinder Singh Rajput
@ 2009-05-25  5:21     ` Avi Kivity
  2009-05-25  5:37       ` Jaswinder Singh Rajput
  0 siblings, 1 reply; 8+ messages in thread
From: Avi Kivity @ 2009-05-25  5:21 UTC (permalink / raw
  To: Jaswinder Singh Rajput; +Cc: Ingo Molnar, x86 maintainers, LKML

Jaswinder Singh Rajput wrote:
>> It's a false alarm.  Isn't there a macro to shut up the warning?
>>
>>     
>
> In arch/x86/kvm/paging_tmpl.h sptep is initialize only in :
>
> for_each_shadow_entry(vcpu, addr, iterator) {
>
> If we skip this, then we end up with wild sptep and returning it.
>
> Do you still think it is a false alarm ?
>   

That loop always has at least one iteration.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH -tip] x86: kvm/paging_tmpl.h intialize the variable before using it
  2009-05-25  5:21     ` Avi Kivity
@ 2009-05-25  5:37       ` Jaswinder Singh Rajput
  2009-05-25  6:01         ` Avi Kivity
  0 siblings, 1 reply; 8+ messages in thread
From: Jaswinder Singh Rajput @ 2009-05-25  5:37 UTC (permalink / raw
  To: Avi Kivity; +Cc: Ingo Molnar, x86 maintainers, LKML

On Mon, 2009-05-25 at 08:21 +0300, Avi Kivity wrote:
> Jaswinder Singh Rajput wrote:
> >> It's a false alarm.  Isn't there a macro to shut up the warning?
> >>
> >>     
> >
> > In arch/x86/kvm/paging_tmpl.h sptep is initialize only in :
> >
> > for_each_shadow_entry(vcpu, addr, iterator) {
> >
> > If we skip this, then we end up with wild sptep and returning it.
> >
> > Do you still think it is a false alarm ?
> >   
> 
> That loop always has at least one iteration.
> 

hmm, In that case we should use a do-while flavor function so that the
compiler will also happy with it.

--
JSR


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH -tip] x86: kvm/paging_tmpl.h intialize the variable before using it
  2009-05-25  5:37       ` Jaswinder Singh Rajput
@ 2009-05-25  6:01         ` Avi Kivity
  0 siblings, 0 replies; 8+ messages in thread
From: Avi Kivity @ 2009-05-25  6:01 UTC (permalink / raw
  To: Jaswinder Singh Rajput; +Cc: Ingo Molnar, x86 maintainers, LKML

Jaswinder Singh Rajput wrote:
>> That loop always has at least one iteration.
>>     
>
> hmm, In that case we should use a do-while flavor function so that the
> compiler will also happy with it.
>   

My own happiness requires that the loop not be rewritten.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH -tip] x86: kvm/paging_tmpl.h intialize the variable before using it
  2009-05-24 12:21 ` Avi Kivity
  2009-05-25  5:17   ` Jaswinder Singh Rajput
@ 2009-05-25  6:05   ` Ingo Molnar
  2009-05-26 11:44     ` Avi Kivity
  1 sibling, 1 reply; 8+ messages in thread
From: Ingo Molnar @ 2009-05-25  6:05 UTC (permalink / raw
  To: Avi Kivity; +Cc: Jaswinder Singh Rajput, x86 maintainers, LKML


* Avi Kivity <avi@redhat.com> wrote:

> Jaswinder Singh Rajput wrote:
>> May be in some cases paging64_fetch() and paging32_fetch() will return sptep
>> without initialization.
>>
>> Also fixes compilation warning:
>>   CC      arch/x86/kernel/io_delay.o
>>  arch/x86/kvm/paging_tmpl.h: In function ‘paging64_fetch’:
>>  arch/x86/kvm/paging_tmpl.h:279: warning: ‘sptep’ may be used uninitialized in this function
>>  arch/x86/kvm/paging_tmpl.h: In function ‘paging32_fetch’:
>>  arch/x86/kvm/paging_tmpl.h:279: warning: ‘sptep’ may be used uninitialized in this function
>>
>> Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
>> ---
>>  arch/x86/kvm/paging_tmpl.h |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
>> index 6bd7020..99cb10d 100644
>> --- a/arch/x86/kvm/paging_tmpl.h
>> +++ b/arch/x86/kvm/paging_tmpl.h
>> @@ -276,7 +276,7 @@ static u64 *FNAME(fetch)(struct kvm_vcpu *vcpu, gva_t addr,
>>  {
>>  	unsigned access = gw->pt_access;
>>  	struct kvm_mmu_page *shadow_page;
>> -	u64 spte, *sptep;
>> +	u64 spte, *sptep = NULL;
>>  	int direct;
>>  	gfn_t table_gfn;
>>  	int r;
>>   
>
> It's a false alarm.  Isn't there a macro to shut up the warning?

there's uninitialized_var(), but that will shut up the warning and 
any _correct_ future warning - so it's dangerous. Initialize it to 
NULL and be done with it? We dont want stray warnings in the kernel.

	Ingo

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH -tip] x86: kvm/paging_tmpl.h intialize the variable before using it
  2009-05-25  6:05   ` Ingo Molnar
@ 2009-05-26 11:44     ` Avi Kivity
  0 siblings, 0 replies; 8+ messages in thread
From: Avi Kivity @ 2009-05-26 11:44 UTC (permalink / raw
  To: Ingo Molnar; +Cc: Jaswinder Singh Rajput, x86 maintainers, LKML

Ingo Molnar wrote:

  

>> It's a false alarm.  Isn't there a macro to shut up the warning?
>>     
>
> there's uninitialized_var(), but that will shut up the warning and 
> any _correct_ future warning - so it's dangerous. Initialize it to 
> NULL and be done with it? We dont want stray warnings in the kernel.
>   

Makes sense, so I've applied the patch.

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2009-05-26 11:45 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-05-20  4:29 [PATCH -tip] x86: kvm/paging_tmpl.h intialize the variable before using it Jaswinder Singh Rajput
2009-05-24 12:21 ` Avi Kivity
2009-05-25  5:17   ` Jaswinder Singh Rajput
2009-05-25  5:21     ` Avi Kivity
2009-05-25  5:37       ` Jaswinder Singh Rajput
2009-05-25  6:01         ` Avi Kivity
2009-05-25  6:05   ` Ingo Molnar
2009-05-26 11:44     ` Avi Kivity

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.