Comment 19 for bug 1270579

Revision history for this message
Andreas Mohr (andi) wrote : Re: Ubuntu should have an upstart job for saving/restoring backlight level on laptops

Commenting on #18:
Checking the brightness script (POSIX-incompatible bashisms etc.), my system says:

# checkbashisms brightness
possible bashism in brightness line 30 (unsafe echo with backslash):
                                echo "\tloading $saved_controller_name"
possible bashism in brightness line 40 (unsafe echo with backslash):
                                echo "\tsaving $controller_name"
possible bashism in brightness line 50 (unsafe echo with backslash):
                                echo "\tCurrent brightness level for $controller_name is $(cat $controller/brightness)"
possible bashism in brightness line 58 (unsafe echo with backslash):
                                        echo "\tSaved brightness level for $saved_controller_name is $(cat $saved_file)"

(should probably prefer using the printf helper for that, since a \t control code might possibly require echo -e, yet availability of sufficiently compatible echo -e possibly isn't guaranteed)

Also some variable uses are not "-quoted, which will cause issues in case of space-containing filesystem items (which is somewhat unlikely to be encountered in the case of firmly Linux-managed /proc or /sys entries, but still...).

And some path strings are openly duplicated rather than assigned to helper variables.

All in all these are fairly minor issues, and I didn't runtime test the script anyway. I simply decided to be bored enough to do a code review ;)