Bug 191361 - comms/smstools3 new rc.d script
Summary: comms/smstools3 new rc.d script
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Many People
Assignee: Guido Falsi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-06-25 10:26 UTC by Pavel Timofeev
Modified: 2014-06-26 22:29 UTC (History)
1 user (show)

See Also:


Attachments
wrong, don't use it (1.20 KB, text/plain)
2014-06-25 10:26 UTC, Pavel Timofeev
no flags Details
new rc.d script itself (622 bytes, application/x-shellscript)
2014-06-25 10:49 UTC, Pavel Timofeev
no flags Details
patch for /usr/ports/comms/smstools3/files/smsd.in (1.02 KB, patch)
2014-06-25 10:50 UTC, Pavel Timofeev
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Timofeev 2014-06-25 10:26:36 UTC
Created attachment 144115 [details]
wrong, don't use it

Hi! Exiting smsd rc.d script doesn't work properly because it has mitakes.
Now it looks such:
.....
. /etc/rc.subr

name="smsd"
rcvar=smsd_enable

load_rc_config ${name}
: ${smsd_enable="NO"}
: ${smsd_pidfile="/var/run/smsd/smsd.pid"}
: ${smsd_infofile="/var/run/smsd/smsd.working"}
: ${smsd_logfile="/var/log/smsd/smsd.log"}
: ${smsd_config="/usr/local/etc/smsd.conf"}
: ${smsd_user="uucp"}
: ${smsd_group="dialer"}

pidfile=${smsd_pidfile}
command="/usr/local/bin/smsd"
command_args="-c${smsd_config} -p${smsd_pidfile} -i${smsd_infofile} -l${smsd_logfile} -u${smsd_user} -g${smsd_group}"

run_rc_command "$1"


It's definetely wrong to use here ${name}_user and ${name}_group to pass it to smsd as arguments.
See /etc/rc.subr, ${name}_user and ${name}_group variables lead to call su to change user.
But smsd has to change user itself using its arguments.

I attached a little bit reworked rc.d script where I deleted smsd_user and smsd_group vars, I deleted smsd_logfile because it's stupid and it may (and has to) be set in config file. Also I added smsd_flags.
It's a root of problem from another PR (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=164197).
Comment 1 Pavel Timofeev 2014-06-25 10:49:03 UTC
Created attachment 144117 [details]
new rc.d script itself
Comment 2 Pavel Timofeev 2014-06-25 10:50:06 UTC
Created attachment 144118 [details]
patch for /usr/ports/comms/smstools3/files/smsd.in
Comment 3 Guido Falsi freebsd_committer freebsd_triage 2014-06-25 13:17:47 UTC
(In reply to timp87 from comment #0)

> It's definetely wrong to use here ${name}_user and ${name}_group to pass it
> to smsd as arguments.
> See /etc/rc.subr, ${name}_user and ${name}_group variables lead to call su
> to change user.
> But smsd has to change user itself using its arguments.

Good catch on this one, I missed this aspect.

> 
> I attached a little bit reworked rc.d script where I deleted smsd_user and
> smsd_group vars, I deleted smsd_logfile because it's stupid

Before going into the technical part, why you have to use such language (stupid) to describe this? It's definitely unkind to me and anyone who has contributed to this port and will not help you with the point you're trying to argue.

> and it may (and has to) be set in config file.

That it "has to" is your personal opinion. The software allows it to be specified both on command line and in configuration file, so it is a matter of personal preference.

Apart from this argument, there is something in your suggestions and I am really evaluating them.

> Also I added smsd_flags.
> It's a root of problem from another PR
> (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=164197).

Just to be clear I don't disagree with your suggestions and the patch, but mainly with the unkind way you expressed the reasons for them.

Apart from this I generally agree with you and am going to test such changes.

These changes constitute a minor POLA, since updating the port after this change could require people to modify their rc.conf files to achieve the same working configuration as before. So I'll also have to cook up an adequate UPDATING message.

Thanks for your patch, I'll keep you updated on this.
Comment 4 Pavel Timofeev 2014-06-25 13:33:52 UTC
> Before going into the technical part, why you have to use such language (stupid) to describe this? It's definitely unkind to me and anyone who has contributed to this port and will not help you with the point you're trying to argue.

Oh, I'm really sorry! =( I wouldn't think that it can offend you! I'm not a native speaker and sometimes I may express in kind of inpolite way, because I don't know how to do it more gently. Sorry again!
Comment 5 Guido Falsi freebsd_committer freebsd_triage 2014-06-26 22:29:07 UTC
Committed, thanks!