Merge lp:~unity-team/nux/nux-fix-810182 into lp:nux/2.0
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Jay Taoko on 2011-11-26 | ||||
Approved revision: | 522 | ||||
Merged at revision: | 522 | ||||
Proposed branch: | lp:~unity-team/nux/nux-fix-810182 | ||||
Merge into: | lp:nux/2.0 | ||||
Diff against target: |
25 lines (+13/-2) 1 file modified
tools/unity_support_test.c (+13/-2) |
||||
To merge this branch: | bzr merge lp:~unity-team/nux/nux-fix-810182 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Tim Penhey (community) | 2011-11-22 | Approve on 2011-11-24 | |
Review via email:
|
Description of the change
Many of the duplicates in bug #810182, report errors with loading of GLX.
extension "GLX" missing on display ":0".
Xlib: extension "GLX" missing on display ":0".
Xlib: extension "GLX" missing on display ":0".
Many of the Xorg logs also report error in loading of the Nvidia or Nouveau driver. I will add a test to detect the presence of GLX on the system. If the system does not have GLX support, then unity_support_test will not continue further and the test will fail (but not crash).
Tim Penhey (thumper) wrote : | # |
Do we have hardware in the QA lab we can use to test this I wonder?
Tim Penhey (thumper) wrote : | # |
Are there any tests around this now?
Jay Taoko (jaytaoko) wrote : | # |
> Hi Jay,
>
> I'd keep the display check first, that way you don't have to check it multiple
> times.
>
> Instead of the explicit checks, can we just use !?
>
> if (!display || !glx_supported) {
> if (!display)
> results.error = strdup ("unable to open display");
> ...
Will do.
> But the code looks sound.
The fix here is meant to do something that must be done before the program can continue. The program is using GLX without testing that it is available on the system. In this case, there is no need for a test. Or rather the "opposite test" is that the programs crashes when we do not check for GLX, as evidenced by the bug reports.
Hi Jay,
I'd keep the display check first, that way you don't have to check it multiple
times.
Instead of the explicit checks, can we just use !?
if (!display || !glx_supported) {
if (!display)
results.error = strdup ("unable to open display");
...
But the code looks sound.