Bug 135614

Summary: xosview-1.8.2-5: local variable used before set
Product: [openSUSE] SUSE LINUX 10.0 Reporter: David Binderman <dcb314>
Component: BasesystemAssignee: Dr. Werner Fink <werner>
Status: RESOLVED INVALID QA Contact: E-mail List <qa-bugs>
Severity: Minor    
Priority: P5 - None    
Version: Final   
Target Milestone: ---   
Hardware: All   
OS: SuSE Linux 10.0   
Whiteboard:
Found By: Other Services Priority:
Business Priority: Blocker: ---
Marketing QA Status: --- IT Deployment: ---

Description David Binderman 2005-11-27 18:07:35 UTC
I just tried to compile package xosview-1.8.2-5 with the Intel C compiler

It said

xwin.cc(124): warning #592: variable "background_pixmap" is used before its value is set

The source code is

        XSetWindowBackgroundPixmap(display_,window_,background_pixmap);

I have read the source code and I agree with the compiler.
Suggest initialise local variable "background_pixmap" before first use.

The email address of the author [ romberg@md.fsl.noaa.gov ] seems to be
broken.
Comment 1 Dr. Werner Fink 2005-11-29 15:20:05 UTC
Wrong due:

#ifdef HAVE_XPM
  doPixmap=getPixmap(&background_pixmap);
#endif

as you can see doPixmap is initalized with 0
and as long as the above line is not executed
the line will not executed.
        
Comment 2 David Binderman 2005-11-30 09:28:01 UTC
(In reply to comment #1)
> #ifdef HAVE_XPM
>   doPixmap=getPixmap(&background_pixmap);
> #endif

Thanks - I did see that code.

Belt and braces, suggest add line of code before the #ifdef
to initialise background_pixmap.

I didn't find out if the macro HAVE_XPM was defined or not.


Comment 3 Dr. Werner Fink 2005-11-30 10:22:17 UTC
Hmmm ... AFAIS even if the cpp macro HAVE_XPM is not defined
the doPixmap stays at 0 and therefore the background_pixmap
will not touched.  Don't know why the compiler can not detect
this.
Comment 4 David Binderman 2005-11-30 18:05:35 UTC
>if the cpp macro HAVE_XPM is not defined
>the doPixmap stays at 0 

I'd be interested to find out how you think this is true.

My memory is failing through the 167 bugs I've found in Suse 
Linux 10.0, but having a wild stab in the dark,
I seem to remember that local variables aren't initialised by 
default in C. This might, or might not, be relevant.
Comment 5 Dr. Werner Fink 2005-11-30 18:12:47 UTC
Let's have a look:

  void XWin::init( int argc, char **argv ){
    XGCValues            gcv;
    XSetWindowAttributes xswa;
    Pixmap               background_pixmap;
    int                  doPixmap = 0;

    setFont();
    setColors();
    getGeometry();
  #ifdef HAVE_XPM
    doPixmap=getPixmap(&background_pixmap);
  #endif

  [...]

    // If there is a pixmap file, set it as the background
    if(doPixmap)
    {
          XSetWindowBackgroundPixmap(display_,window_,background_pixmap);
    }
 
  [...]

AFAIS the doPixmap is initialized with 0 whereas background_pixmap
is not initialized at all ... but the acces to background_pixmap
is only alowd if and only if doPixmap is not 0, that means that
the getPixmap() function has set the background_pixmap to something
valid.

If this is not true please reopen the bug and correct me.
Comment 6 David Binderman 2005-11-30 18:16:20 UTC
>If this is not true please reopen the bug and correct me.

I stand corrected.

Thanks for trowelling it on ;->