Closed Bug 334782 Opened 14 years ago Closed 14 years ago

potential uninitialized x in nsWindow::WidgetToScreen


(Core :: Widget, defect)

Not set





(Reporter: guninski, Assigned: bugzilla)


(Blocks 1 open bug, )


(Keywords: coverity)


(1 file, 1 obsolete file)

coverity cid 255 suspects x may be used uninitialized here:

1269 nsWindow::WidgetToScreen(const nsRect& aOldRect, nsRect& aNewRect)
1270 {
1271     gint x, y = 0;
1273     if (mContainer) {
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) {
1279         gdk_window_get_origin(mDrawingarea->inner_window, &x, &y);
1280         LOG(("WidgetToScreen (drawing) %d %d\n", x, y));
1281     }
1283     aNewRect.x = x + aOldRect.x;
1284     aNewRect.y = y + aOldRect.y;
1285     aNewRect.width = aOldRect.width;
1286     aNewRect.height = aOldRect.height;
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
Assignee: nobody → general
Component: General → Widget
Product: Firefox → Core
QA Contact: general
georgi: please use bonsai links instead.
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
(In reply to comment #1)
> georgi: please use bonsai links instead.

ok, got it.
I haven't found anything better than and

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.
Attached patch init x and ySplinter Review
Initialize x and y to 0
Attached patch Don't init x and y (obsolete) — Splinter Review
Don't initialize x and y
Comment on attachment 237966 [details] [diff] [review]
init x and y

thanks! get checkin help on IRC
Attachment #237966 - Flags: superreview+
Attachment #237966 - Flags: review+
Attachment #237967 - Attachment is obsolete: true
Assignee: general → aqualon
Checked in.
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.