334782 - potential uninitialized x in nsWindow::WidgetToScreen

Status: RESOLVED FIXED

(Core :: Widget, defect)

Description

[georgi - hopefully not receiving bugspam]

coverity cid 255 suspects x may be used uninitialized here:

1269 nsWindow::WidgetToScreen(const nsRect& aOldRect, nsRect& aNewRect)
1270 {
1271     gint x, y = 0;
^^^^^^^^^^^^^^^
1272 
1273     if (mContainer) {
^^^^^^^^^^^^^false
1274         gdk_window_get_root_origin(GTK_WIDGET(mContainer)->window,
1275                                    &x, &y);
1276         LOG(("WidgetToScreen (container) %d %d\n", x, y));
1277     }
1278     else if (mDrawingarea) {
^^^^^^^^^^^^false
1279         gdk_window_get_origin(mDrawingarea->inner_window, &x, &y);
1280         LOG(("WidgetToScreen (drawing) %d %d\n", x, y));
1281     }
1282 
1283     aNewRect.x = x + aOldRect.x;
^^^^^^^^^^^^^^^^^^^^^^^^
1284     aNewRect.y = y + aOldRect.y;
1285     aNewRect.width = aOldRect.width;
1286     aNewRect.height = aOldRect.height;
1287 
1288     return NS_OK;
1289 }

not sure if this codepath may be hit, but it wouldn't hurt doing "x = 0"

in case coverity is wrong, "y = 0" is just wasting cpu cycles and it is not very consistent to initialize only one of x,y

Comment 1

[timeless]

georgi: please use bonsai links instead.
steps:
1. find your nice lxr link.
2. click the cvsblame link at the top right. this will pin your file to the specific cvs rev.
3. add &mark=line,line,line-line,line
4. add #line

Comment 2

[georgi - hopefully not receiving bugspam]

(In reply to comment #1)
> georgi: please use bonsai links instead.

ok, got it.

Comment 3

[Bruno 'Aqualon' Escherl]

I haven't found anything better than http://maemo.org/lxr/source/gtk%2B/gdk/x11/gdkwindow-x11.c#3077 and http://maemo.org/lxr/source/gtk%2B/gdk/x11/gdkwindow-x11.c#3201

I'm no GTK expert, but both gdk_window_get_root_origin and gdk_window_get_origin can fail before setting x and y to a value. The question is now, what consequences a failure in these functions has for nsWindow anyway.

Another possibility to have x not initialized is, when neither mContainer nor mDrawingarea are initialized. Don't know if this can happen at all.

I would say, that initializing x to 0 is the secure way to fix this, but can't judge, if not initializing x and y would be ok too.

Comment 4

[Bruno 'Aqualon' Escherl]

Attachment: init x and y

Initialize x and y to 0

Comment 5

[Bruno 'Aqualon' Escherl]

Attachment: Don't init x and y

Don't initialize x and y

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 237966 [details] [diff] [review]
init x and y

thanks! get checkin help on IRC

Comment 7

[Boris Zbarsky [:bzbarsky]]

Checked in.