Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=700571 --- Comment #5 from Hans de Goede <hdegoede@xxxxxxxxxx> 2011-05-06 05:22:13 EDT --- Much better now, almost there. Needs work: - You're installing the example config file as /etc/spindown.conf now, no need to include at is as %doc then - Thanks for doing the new initscript, but I'm afraid it needs some work: Must Fix: - The LSB HEADER is entirely empty, fill in the Short Description and Description and remove all the other lines. - statuspipe="/tmp/spindown.status" Is vulnerable to symlinks attacks, please make this /var/run/spindown.status instead - the $prog in "status -p /var/run/spindownd.pid $prog" must be $exec - start() is missing a "return $retval" at the end Should fix: - Given that you've effectively written a new script, please add it as Source1: (without an url) and then in $setup do: cp -p %{SOURCE1} . Rather then making it a hard to read patch. This will also make making future changes to it a lot easier. - Drop the unused RETVAL variable - Please use a pidfile variable (like lockfile and statuspipe) rather then writing out /var/run/spindownd.pid in full everywhere - This part: echo -n $"Starting $prog: " if [ $UID -ne 0 ] ; then failure echo -e "\nUser has insufficient privilege." exit 4 fi if [ ! -x $exec ]; then failure echo -e "\nDaemon binary not executable." exit 5 fi if [ ! -f $config ]; then failure echo -e "\nConfig file missing." exit 6 fi Deviates from how most Fedora init scripts do it, please change this to: [ $UID -eq 0 ] || exit 4 [ -x $exec ] || exit 5 [ -f $config ] || exit 6 echo -n $"Starting $prog: " - You can likely (and if it works should) write this bit: $exec -d -f $statuspipe -c $config -p /var/run/spindownd.pid; retval=$? [ "$retval" -eq 0 ] && success || failure echo As: daemon $exec -d -f $statuspipe -c $config -p /var/run/spindownd.pid; retval=$? echo - stop() can be simplified to just: stop() { echo -n $"Stopping $prog: " killproc -p /var/run/spindownd.pid $exec retval=$? echo [ $retval -eq 0 ] && rm -f $lockfile return $retval } - judging from the old initscript reload should be: reload() { killproc -p /var/run/spindownd.pid $exec -HUP } And force_reload should then call reload rather then restart -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review