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 ;)

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/1270579

Title:
  Ubuntu should have an upstart job for saving/restoring backlight level
  on laptops

To manage notifications about this bug go to:
https://bugs.launchpad.net/elementaryos/+bug/1270579/+subscriptions

-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Reply via email to