Bug 96215 - [regression] keyboard backlight is not exported as per commit 793642bfb
Summary: [regression] keyboard backlight is not exported as per commit 793642bfb
Status: RESOLVED FIXED
Alias: None
Product: upower
Classification: Unclassified
Component: general (show other bugs)
Version: unspecified
Hardware: All All
: medium major
Assignee: Marco Trevisan (Treviño)
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-05-25 15:41 UTC by Marco Trevisan (Treviño)
Modified: 2016-05-25 16:48 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
UpKbdBacklight: don't check the *end value when using g_ascii_strtoll (1008 bytes, patch)
2016-05-25 15:47 UTC, Marco Trevisan (Treviño)
Details | Splinter Review

Description Marco Trevisan (Treviño) 2016-05-25 15:41:40 UTC
With last upower:
   (upowerd:12806): UPower-WARNING **: failed to convert brightness: 2

This is because of the "*end != '\0'" check which fails since g_ascii_strtoll returns the position of the last parsed element, and thus that condition is valid only for *(end+1), which is something I would avoid to check as the fact that end == buf is already enough to check that the parsing went ok (as per g_ascii_strtoll docs).
Comment 1 Marco Trevisan (Treviño) 2016-05-25 15:47:13 UTC
Created attachment 124085 [details] [review]
UpKbdBacklight: don't check the *end value when using g_ascii_strtoll

g_ascii_strtoll would set end to match buf when a parsing error occurred,
so there's no reason to also check what this is pointing to
Comment 2 Martin Pitt 2016-05-25 16:40:37 UTC
Thanks, applied. I had assumed that you tested the previous patch on a system that actually has a keyboard backlight (I don't).

> end == buf is already enough to check that the parsing went ok (as per g_ascii_strtoll docs).

ITYM s/ok/not ok/. What does this stumble on, I take it it is the newline? I. e. it's trying to parse '2\n' and *end == '\n'?
Comment 3 Marco Trevisan (Treviño) 2016-05-25 16:48:19 UTC
(In reply to Martin Pitt from comment #2)
> Thanks, applied. I had assumed that you tested the previous patch on a
> system that actually has a keyboard backlight (I don't).

Oh, well actually I did... But maybe it happened for some reason that I forgot to fully test the case after adding this *end check you suggested. Maybe I just didn't run the right binary, sorry about that.

> > end == buf is already enough to check that the parsing went ok (as per g_ascii_strtoll docs).
> 
> ITYM s/ok/not ok/.

Yeah... or s/that/whether/

> What does this stumble on, I take it it is the newline?
> I. e. it's trying to parse '2\n' and *end == '\n'?

Yep, *end is 10, and thus '\n'.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.