Bug 96215

Summary:	[regression] keyboard backlight is not exported as per commit 793642bfb
Product:	upower	Reporter:	Marco Trevisan (Treviño) <mail>
Component:	general	Assignee:	Marco Trevisan (Treviño) <mail>
Status:	RESOLVED FIXED	QA Contact:
Severity:	major
Priority:	medium	CC:	mail, martin.pitt
Version:	unspecified
Hardware:	All
OS:	All
Whiteboard:
i915 platform:		i915 features:
Attachments:	UpKbdBacklight: don't check the *end value when using g_ascii_strtoll

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.