On Tue, 11.03.14 18:55, Josh Triplett ([email protected]) wrote:
> + /* Some systems turn the backlight all the way off at the
> + * lowest levels. Clamp saved brightness to at least 1 or 5%
> + * of max_brightness. This avoids preserving an unreadably
> dim
> + * screen, which would otherwise force the user to disable
> + * state restoration.
> + */
> + max_brightness_str = udev_device_get_sysattr_value(device,
> "max_brightness");
> + if (!max_brightness_str) {
> + log_warning("Failed to read max_brightness
> attribute; not checking saved brightness");
> + } else {
We try to avoid unnnecessary {} for single-line if blocks, if we can...
Hmmm, could you maybe move the entire clamping thing into a function of
its own? Maybe clamp_brightness(struct udev_device *d, char
**brightness) or so, that simply patches the brightness string if it
feels like it, otherwise leaves it unmodified?
> + unsigned long long brightness,
> max_brightness, new_brightness;
Wow, you expect a lot of brightness levels! ;-)
I'd just stick to "unsigned" here... Keeps it more readable...
> +
> + r = safe_atollu(value, &brightness);
> + if (r < 0) {
> + log_error("Failed to parse brightness
> \"%s\": %s", value, strerror(-r));
> + return EXIT_FAILURE;
> + }
> +
> + r = safe_atollu(max_brightness_str, &max_brightness);
> + if (r < 0) {
> + log_error("Failed to parse max_brightness
> \"%s\": %s", max_brightness_str, strerror(-r));
> + return EXIT_FAILURE;
> + }
Hmm, I'd prefer if the whole clamping business would be
non-fatal. i.e. it clamps if it can read the files and parse them, but
if it can't it won't do anything...
> +
> + new_brightness = MAX3(brightness, 1,
> max_brightness/20);
> + if (new_brightness != brightness) {
> + _cleanup_free_ char *old_value = value;
> +
> + value = asprintf("%llu",
> new_brightness);
asprintf() works differently... r = asprintf(&value, "%llu", new_brightness)...
Lennart
--
Lennart Poettering, Red Hat
_______________________________________________
systemd-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
