Summary: | [GTK] Crashes if DISPLAY is unset | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alberto Garcia <berto> | ||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | bugs-noreply, cgarcia, kapouer, mcatanzaro | ||||||
Priority: | P2 | ||||||||
Version: | Other | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
See Also: |
http://bugs.debian.org/803104 https://bugzilla.gnome.org/show_bug.cgi?id=757061 |
||||||||
Attachments: |
|
Description
Alberto Garcia
2016-04-25 04:49:42 PDT
Created attachment 277239 [details]
Patch
Comment on attachment 277239 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=277239&action=review > Source/WebCore/platform/graphics/x11/PlatformDisplayX11.cpp:52 > - if (m_ownedDisplay) > + if (m_ownedDisplay && m_display) what about initializing m_ownedDisplay to false if display is NULL? we can't own a display that doesn't exist. Created attachment 277247 [details]
Patch
Ok, I chose to initialize m_ownedDisplay in the body of the constructor to make sure that m_display is set first. Otherwise we'd have to rely on the order in which m_display and m_ownedDisplay are declared in the class definition.
(In reply to comment #3) > Created attachment 277247 [details] > Patch > > Ok, I chose to initialize m_ownedDisplay in the body of the constructor to > make sure that m_display is set first. Otherwise we'd have to rely on the > order in which m_display and m_ownedDisplay are declared in the class > definition. Isn't the order ensured by the compiler? why do we have a warning when the order in the list doesn't match the one in the declaration, then? Comment on attachment 277247 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=277247&action=review > Source/WebCore/platform/graphics/x11/PlatformDisplayX11.cpp:41 > + m_ownedDisplay = m_display != nullptr; We don't explicitly check nullptr, m_ownedDisplay = m_display; should just work, I think. (In reply to comment #4) > Isn't the order ensured by the compiler? why do we have a warning when the > order in the list doesn't match the one in the declaration, then? Member variables are always initialized in the order they are declared in the class, NOT the order they are listed in the initializer list. Putting them out of order in the initializer list is thus very confusing, and worth warning about. We should never do that in WebKit. Since relying on the order of declarations is risky -- someone could easily swap those in a "cleanup" and not notice the warning -- I agree with Berto that it's a good idea to do the assignment in the body of the constructor. Committed r200046: <http://trac.webkit.org/changeset/200046> |