* [PATCH 3/3] staging: xgifb: Initialize uninitialized variables
@ 2010-09-07 5:34 Javier Martinez Canillas
2010-09-07 8:46 ` Jiri Slaby
0 siblings, 1 reply; 5+ messages in thread
From: Javier Martinez Canillas @ 2010-09-07 5:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Bill Pemberton, Arnaud Patard, Randy Dunlap,
devel, linux-kernel
Current patch solves compilation warnings in staging/xgifb for using possibly uninitialized variables.
Signed-off-by: Javier Martinez Canillas <martinez.javier@gmail.com>
---
drivers/staging/xgifb/vb_setmode.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/xgifb/vb_setmode.c b/drivers/staging/xgifb/vb_setmode.c
index c4db2fc..720a592 100644
--- a/drivers/staging/xgifb/vb_setmode.c
+++ b/drivers/staging/xgifb/vb_setmode.c
@@ -376,7 +376,7 @@ void InitTo330Pointer(unsigned char ChipType, struct vb_device_info *pVBInfo)
unsigned char XGISetModeNew(struct xgi_hw_device_info *HwDeviceExtension,
unsigned short ModeNo)
{
- unsigned short ModeIdIndex;
+ unsigned short ModeIdIndex = 0;
/* unsigned char *pVBInfo->FBAddr = HwDeviceExtension->pjVideoMemoryAddress; */
struct vb_device_info VBINF;
struct vb_device_info *pVBInfo = &VBINF;
@@ -3834,7 +3834,7 @@ unsigned char XGI_SetCRT2Group301(unsigned short ModeNo,
struct xgi_hw_device_info *HwDeviceExtension,
struct vb_device_info *pVBInfo)
{
- unsigned short tempbx, ModeIdIndex, RefreshRateTableIndex;
+ unsigned short tempbx, ModeIdIndex = 0, RefreshRateTableIndex;
tempbx = pVBInfo->VBInfo;
pVBInfo->SetFlag |= ProgrammingCRT2;
--
1.7.0.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 3/3] staging: xgifb: Initialize uninitialized variables
2010-09-07 5:34 [PATCH 3/3] staging: xgifb: Initialize uninitialized variables Javier Martinez Canillas
@ 2010-09-07 8:46 ` Jiri Slaby
2010-09-08 0:38 ` Greg KH
0 siblings, 1 reply; 5+ messages in thread
From: Jiri Slaby @ 2010-09-07 8:46 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: Greg Kroah-Hartman, Bill Pemberton, Arnaud Patard, Randy Dunlap,
devel, linux-kernel
On 09/07/2010 07:34 AM, Javier Martinez Canillas wrote:
> Current patch solves compilation warnings in staging/xgifb for using possibly uninitialized variables.
>
> Signed-off-by: Javier Martinez Canillas <martinez.javier@gmail.com>
> ---
> drivers/staging/xgifb/vb_setmode.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/xgifb/vb_setmode.c b/drivers/staging/xgifb/vb_setmode.c
> index c4db2fc..720a592 100644
> --- a/drivers/staging/xgifb/vb_setmode.c
> +++ b/drivers/staging/xgifb/vb_setmode.c
> @@ -376,7 +376,7 @@ void InitTo330Pointer(unsigned char ChipType, struct vb_device_info *pVBInfo)
> unsigned char XGISetModeNew(struct xgi_hw_device_info *HwDeviceExtension,
> unsigned short ModeNo)
> {
> - unsigned short ModeIdIndex;
> + unsigned short ModeIdIndex = 0;
> /* unsigned char *pVBInfo->FBAddr = HwDeviceExtension->pjVideoMemoryAddress; */
> struct vb_device_info VBINF;
> struct vb_device_info *pVBInfo = &VBINF;
No, please don't. This is a compiler bug.
The first use in that function is:
XGI_SearchModeID( ModeNo , &ModeIdIndex, pVBInfo );
and that function contains:
if (XYZ) {
for( *ModeIdIndex = 0 ; ; ( *ModeIdIndex )++ )
...
} else {
for( *ModeIdIndex = 0 ; ; ( *ModeIdIndex )++ )
...
}
So your compiler is foolish if that emits a warning.
> @@ -3834,7 +3834,7 @@ unsigned char XGI_SetCRT2Group301(unsigned short ModeNo,
> struct xgi_hw_device_info *HwDeviceExtension,
> struct vb_device_info *pVBInfo)
> {
> - unsigned short tempbx, ModeIdIndex, RefreshRateTableIndex;
> + unsigned short tempbx, ModeIdIndex = 0, RefreshRateTableIndex;
>
> tempbx = pVBInfo->VBInfo;
> pVBInfo->SetFlag |= ProgrammingCRT2;
The same here.
regards,
--
js
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 3/3] staging: xgifb: Initialize uninitialized variables
2010-09-07 8:46 ` Jiri Slaby
@ 2010-09-08 0:38 ` Greg KH
2010-09-08 7:16 ` Jiri Slaby
0 siblings, 1 reply; 5+ messages in thread
From: Greg KH @ 2010-09-08 0:38 UTC (permalink / raw)
To: Jiri Slaby
Cc: Javier Martinez Canillas, Bill Pemberton, Arnaud Patard,
Randy Dunlap, devel, linux-kernel
On Tue, Sep 07, 2010 at 10:46:27AM +0200, Jiri Slaby wrote:
> On 09/07/2010 07:34 AM, Javier Martinez Canillas wrote:
> > Current patch solves compilation warnings in staging/xgifb for using possibly uninitialized variables.
> >
> > Signed-off-by: Javier Martinez Canillas <martinez.javier@gmail.com>
> > ---
> > drivers/staging/xgifb/vb_setmode.c | 4 ++--
> > 1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/xgifb/vb_setmode.c b/drivers/staging/xgifb/vb_setmode.c
> > index c4db2fc..720a592 100644
> > --- a/drivers/staging/xgifb/vb_setmode.c
> > +++ b/drivers/staging/xgifb/vb_setmode.c
> > @@ -376,7 +376,7 @@ void InitTo330Pointer(unsigned char ChipType, struct vb_device_info *pVBInfo)
> > unsigned char XGISetModeNew(struct xgi_hw_device_info *HwDeviceExtension,
> > unsigned short ModeNo)
> > {
> > - unsigned short ModeIdIndex;
> > + unsigned short ModeIdIndex = 0;
> > /* unsigned char *pVBInfo->FBAddr = HwDeviceExtension->pjVideoMemoryAddress; */
> > struct vb_device_info VBINF;
> > struct vb_device_info *pVBInfo = &VBINF;
>
> No, please don't. This is a compiler bug.
>
> The first use in that function is:
> XGI_SearchModeID( ModeNo , &ModeIdIndex, pVBInfo );
>
> and that function contains:
> if (XYZ) {
> for( *ModeIdIndex = 0 ; ; ( *ModeIdIndex )++ )
> ...
> } else {
> for( *ModeIdIndex = 0 ; ; ( *ModeIdIndex )++ )
> ...
> }
>
> So your compiler is foolish if that emits a warning.
No, I don't see a way that the compiler could really figure that type of
logic out. It doesn't know that XGI_SearchModeID() always will set that
field. Heck, at first glance I couldn't even figure it out :)
So I don't mind applying this patch, as I'll just constantly keep
getting people sending me the same change, when they could be off
working on other things that this driver needs.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 3/3] staging: xgifb: Initialize uninitialized variables
2010-09-08 0:38 ` Greg KH
@ 2010-09-08 7:16 ` Jiri Slaby
2010-09-08 8:49 ` Greg KH
0 siblings, 1 reply; 5+ messages in thread
From: Jiri Slaby @ 2010-09-08 7:16 UTC (permalink / raw)
To: Greg KH
Cc: Javier Martinez Canillas, Bill Pemberton, Arnaud Patard,
Randy Dunlap, devel, linux-kernel
On 09/08/2010 02:38 AM, Greg KH wrote:
> On Tue, Sep 07, 2010 at 10:46:27AM +0200, Jiri Slaby wrote:
>> On 09/07/2010 07:34 AM, Javier Martinez Canillas wrote:
>>> Current patch solves compilation warnings in staging/xgifb for using possibly uninitialized variables.
>>>
>>> Signed-off-by: Javier Martinez Canillas <martinez.javier@gmail.com>
>>> ---
>>> drivers/staging/xgifb/vb_setmode.c | 4 ++--
>>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/staging/xgifb/vb_setmode.c b/drivers/staging/xgifb/vb_setmode.c
>>> index c4db2fc..720a592 100644
>>> --- a/drivers/staging/xgifb/vb_setmode.c
>>> +++ b/drivers/staging/xgifb/vb_setmode.c
>>> @@ -376,7 +376,7 @@ void InitTo330Pointer(unsigned char ChipType, struct vb_device_info *pVBInfo)
>>> unsigned char XGISetModeNew(struct xgi_hw_device_info *HwDeviceExtension,
>>> unsigned short ModeNo)
>>> {
>>> - unsigned short ModeIdIndex;
>>> + unsigned short ModeIdIndex = 0;
>>> /* unsigned char *pVBInfo->FBAddr = HwDeviceExtension->pjVideoMemoryAddress; */
>>> struct vb_device_info VBINF;
>>> struct vb_device_info *pVBInfo = &VBINF;
>>
>> No, please don't. This is a compiler bug.
>>
>> The first use in that function is:
>> XGI_SearchModeID( ModeNo , &ModeIdIndex, pVBInfo );
>>
>> and that function contains:
>> if (XYZ) {
>> for( *ModeIdIndex = 0 ; ; ( *ModeIdIndex )++ )
>> ...
>> } else {
>> for( *ModeIdIndex = 0 ; ; ( *ModeIdIndex )++ )
>> ...
>> }
>>
>> So your compiler is foolish if that emits a warning.
>
> No, I don't see a way that the compiler could really figure that type of
> logic out. It doesn't know that XGI_SearchModeID() always will set that
> field. Heck, at first glance I couldn't even figure it out :)
No, the compiler could. Trust me :). Actually you've just hidden a bug
in XGI_SearchModeID. There is #ifdef LINUX and LINUX is defined in
main.h. But that file is not included in vb_setmode.c. Hence the
uninitialized warnings...
> So I don't mind applying this patch, as I'll just constantly keep
> getting people sending me the same change, when they could be off
> working on other things that this driver needs.
So instead of hiding bugs, please fix the right ones.
thanks,
--
js
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 3/3] staging: xgifb: Initialize uninitialized variables
2010-09-08 7:16 ` Jiri Slaby
@ 2010-09-08 8:49 ` Greg KH
0 siblings, 0 replies; 5+ messages in thread
From: Greg KH @ 2010-09-08 8:49 UTC (permalink / raw)
To: Jiri Slaby
Cc: Javier Martinez Canillas, Bill Pemberton, Arnaud Patard,
Randy Dunlap, devel, linux-kernel
On Wed, Sep 08, 2010 at 09:16:43AM +0200, Jiri Slaby wrote:
> On 09/08/2010 02:38 AM, Greg KH wrote:
> > On Tue, Sep 07, 2010 at 10:46:27AM +0200, Jiri Slaby wrote:
> >> On 09/07/2010 07:34 AM, Javier Martinez Canillas wrote:
> >>> Current patch solves compilation warnings in staging/xgifb for using possibly uninitialized variables.
> >>>
> >>> Signed-off-by: Javier Martinez Canillas <martinez.javier@gmail.com>
> >>> ---
> >>> drivers/staging/xgifb/vb_setmode.c | 4 ++--
> >>> 1 files changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/staging/xgifb/vb_setmode.c b/drivers/staging/xgifb/vb_setmode.c
> >>> index c4db2fc..720a592 100644
> >>> --- a/drivers/staging/xgifb/vb_setmode.c
> >>> +++ b/drivers/staging/xgifb/vb_setmode.c
> >>> @@ -376,7 +376,7 @@ void InitTo330Pointer(unsigned char ChipType, struct vb_device_info *pVBInfo)
> >>> unsigned char XGISetModeNew(struct xgi_hw_device_info *HwDeviceExtension,
> >>> unsigned short ModeNo)
> >>> {
> >>> - unsigned short ModeIdIndex;
> >>> + unsigned short ModeIdIndex = 0;
> >>> /* unsigned char *pVBInfo->FBAddr = HwDeviceExtension->pjVideoMemoryAddress; */
> >>> struct vb_device_info VBINF;
> >>> struct vb_device_info *pVBInfo = &VBINF;
> >>
> >> No, please don't. This is a compiler bug.
> >>
> >> The first use in that function is:
> >> XGI_SearchModeID( ModeNo , &ModeIdIndex, pVBInfo );
> >>
> >> and that function contains:
> >> if (XYZ) {
> >> for( *ModeIdIndex = 0 ; ; ( *ModeIdIndex )++ )
> >> ...
> >> } else {
> >> for( *ModeIdIndex = 0 ; ; ( *ModeIdIndex )++ )
> >> ...
> >> }
> >>
> >> So your compiler is foolish if that emits a warning.
> >
> > No, I don't see a way that the compiler could really figure that type of
> > logic out. It doesn't know that XGI_SearchModeID() always will set that
> > field. Heck, at first glance I couldn't even figure it out :)
>
> No, the compiler could. Trust me :). Actually you've just hidden a bug
> in XGI_SearchModeID. There is #ifdef LINUX and LINUX is defined in
> main.h. But that file is not included in vb_setmode.c. Hence the
> uninitialized warnings...
Doh, you are right.
> > So I don't mind applying this patch, as I'll just constantly keep
> > getting people sending me the same change, when they could be off
> > working on other things that this driver needs.
>
> So instead of hiding bugs, please fix the right ones.
Good point, I'll go do that right now.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-09-08 8:56 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-07 5:34 [PATCH 3/3] staging: xgifb: Initialize uninitialized variables Javier Martinez Canillas
2010-09-07 8:46 ` Jiri Slaby
2010-09-08 0:38 ` Greg KH
2010-09-08 7:16 ` Jiri Slaby
2010-09-08 8:49 ` Greg KH
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox