touch-packages team mailing list archive

[Andreas Mohr <andi@xxxxxxxx>]

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
Touch seeded packages, which is subscribed to sysvinit in Ubuntu.
https://bugs.launchpad.net/bugs/1270579

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

Status in elementary OS:
  Confirmed
Status in One Hundred Papercuts:
  Confirmed
Status in Ubuntu MATE:
  Opinion
Status in systemd package in Ubuntu:
  Triaged
Status in sysvinit package in Ubuntu:
  Confirmed
Status in sysvinit package in Debian:
  New

Bug description:
  I have tested many distros - Arch, OpenSuSe 13.1, Fedora 20, ALT Linux p7, Sabayon, Mageia 4.
  Many of them are based on systemd.
  It does not matter what init system they use, but all of them have very usefull script (or binary program, I don't know) for saving and restoring backlight level on laptop.

  In these distros there is a script for save and restore backlight levels for two video-cards (acpi_video0 and acpi_video1). During system boot it is indicated as follows:
  "
  [  OK  ] Started Load/Save Screen Backlight Brightness of acpi_video1.
  [  OK  ] Started Load/Save Screen Backlight Brightness of acpi_video0.
  ".

  I have a draft version, which works normally on my VAIO F13Z1R laptop with Ubuntu 12.04.4. It is in attachment.
  You can use it unmodified or modify it for compatibility or something. I made symlinks for it:
  ln -s /etc/init.d/brightness /etc/rc0.d/S25backlight
  ln -s /etc/init.d/brightness /etc/rcS.d/S25backlight
  ln -s /etc/init.d/brightness /etc/rc6.d/S25backlight

  For details you can read my comment on askubuntu
  (http://askubuntu.com/a/227553/66509).

  I think that having a script for saving and restoring backlight level is very useful for laptop Ubuntu users.
  So, please, add such init script to default Ubuntu installation.

  ProblemType: Bug
  DistroRelease: Ubuntu 14.04
  Package: initscripts 2.88dsf-41ubuntu5
  ProcVersionSignature: Ubuntu 3.13.0-4.19-generic 3.13.0-rc8
  Uname: Linux 3.13.0-4-generic i686
  ApportVersion: 2.13.1-0ubuntu1
  Architecture: i386
  CurrentDesktop: GNOME
  Date: Sun Jan 19 17:20:26 2014
  InstallationDate: Installed on 2013-10-20 (91 days ago)
  InstallationMedia: Ubuntu 13.10 "Saucy Salamander" - Release i386 (20131016.1)
  SourcePackage: sysvinit
  UpgradeStatus: Upgraded to trusty on 2013-11-19 (61 days ago)

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