Nux

Merge lp:~unity-team/nux/nux-fix-810182 into lp:nux/2.0

Proposed by Jay Taoko on 2011-11-22
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
Reviewer Review Type Date Requested Status
Tim Penhey (community) 2011-11-22 Approve on 2011-11-24
Review via email: mp+83057@code.launchpad.net

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).

To post a comment you must log in.
Tim Penhey (thumper) 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");
...

But the code looks sound.

review: Approve
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.

lp:~unity-team/nux/nux-fix-810182 updated on 2011-11-24
522. By Jay Taoko on 2011-11-24

* Minor fix

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'tools/unity_support_test.c'
2--- tools/unity_support_test.c 2011-10-14 16:36:25 +0000
3+++ tools/unity_support_test.c 2011-11-24 16:50:17 +0000
4@@ -720,8 +720,19 @@
5
6 // Open a X11 connection and get the root window.
7 display = XOpenDisplay (display_name);
8- if (display == NULL) {
9- results.error = strdup ("unable to open display");
10+
11+ // Before doing anything with GLX, check that it is supported on the system.
12+ Bool glx_supported = False;
13+ int dummy0, dummy1;
14+ if (display)
15+ glx_supported = glXQueryExtension(display, &dummy0, &dummy1);
16+
17+ if (!display || !glx_supported) {
18+ if (!display)
19+ results.error = strdup ("unable to open display");
20+ else
21+ results.error = strdup ("GLX is not available on the system");
22+
23 // exit with 5, to tell "it's not an error we should cache"
24 results.result = 5;
25 }

Subscribers

People subscribed via source and target branches

to all changes: