Comment 7 for bug 289520

Revision history for this message
Henrik Rydberg (rydberg) wrote :

I presume you are talking about patch 91.

The current code is executing the body of the for loop for both end points, cur and shared_value_abs, Besides being unnecessary, it means the loop exits for a value outside of the valid range. This makes it impossible to use the guarded_up/down functions, which never leaves the valid range.

It *is* possible to make the patch smaller, by 18 lines, by leaving the if statements untouched, like this:

@@ -331,11 +322,12 @@
                return TRUE;
        }

- /* step the correct way */
+ /* step the correct way - by using guarded and scaled stepping */
        if (cur < shared_value_abs) {
                /* going up */
- for (i=cur; i<=shared_value_abs; i++) {
- ret = gpm_brightness_xrandr_output_set_internal (brightness, output, i);
+ while (cur < shared_value_abs) {
+ cur = gpm_brightness_get_guarded_up (min, cur, shared_value_abs);
+ ret = gpm_brightness_xrandr_output_set_internal (brightness, output, cur);
                        if (!ret) {
                                break;
                        }
@@ -344,9 +336,10 @@
                        }
                }
        } else {
- /* going down */
- for (i=cur; i>=shared_value_abs; i--) {
- ret = gpm_brightness_xrandr_output_set_internal (brightness, output, i);
+ while (cur > shared_value_abs) {
+ /* going down */
+ cur = gpm_brightness_get_guarded_down (shared_value_abs, cur, max);
+ ret = gpm_brightness_xrandr_output_set_internal (brightness, output, cur);
                        if (!ret) {
                                break;
                        }

However, the resulting code looks rather artifical. Would you prefer a patch base on the above?