svn commit: r424939 - in head/sysutils/cbsd: . files

[Jason E. Hale]

On Sun, Oct 30, 2016 at 5:39 AM, Mathieu Arnold <mat at freebsd.org> wrote:
> Le 30/10/2016 à 10:22, Jason E. Hale a écrit :
>>
>>  cbsdd_stop()
>>  {
>> -     [ -f "${pidfile}" ] && kill -9 $( /bin/cat ${pidfile} )
>> +     if [ -f "${pidfile}" ]; then
>> +             kill -9 $( /bin/cat ${pidfile} )
>
> This should be pkill -9 -f ${pidfile}
>
>> +             /bin/rm -f ${pidfile}
>> +     fi
>>  }
>>
>>  cbsdd_reload()
>>  {
>> -     [ -f "${pidfile}" ] && kill -9 $( /bin/cat ${pidfile} ) > /dev/null 2>&1
>> +     if [ -f "${pidfile}" ]; then
>> +             kill -9 $( /bin/cat ${pidfile} ) > /dev/null 2>&1
>> +             /bin/rm -f ${pidfile}
>> +     fi
>>       run_rc_command "start"
>
> I think this should simply be run_rc_command "restart" but then, it is
> semantically wrong, reload means tell the process "hey, reload your
> config" or something, not stop/start, that is restart :-)
>
>>       exit 0
>>  }
>>
>> +cbsdd_status()
>> +{
>> +     if [ -f "${pidfile}" ]; then
>> +             pids=$( /bin/cat ${pidfile} )
>> +             echo "${name} is running as pid ${pids}."
>> +     else
>> +             echo "${name} is not running."
>> +             return 1
>> +     fi
>> +}
>
> This is somewhat wrong, it checks that the pid file exists, but this
> file existing does not mean the process is there, it should at least be
> something like:
>
> if [ -f "${pidfile}" ] && pgrep -F ${pidfile} -q;
>
>
>>  run_rc_command "$1"
>>

I've forwarded your message to the maintainer, and I'll see what he
has to say about it since he made these changes upstream as well.

-Jason