Skip to contentSkip to conversation

inadyn: fix init script and default config file

PATCH New Thomas Petazzoni 3 participants 7 messages Jul 5, 2015 – Jul 14, 2015

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 inadyn: fix init script and default config file

This commit does a number of fixes to the inadyn package to make it work properly "out of the box":

* inadyn is installed in /usr/sbin, not /usr/bin, so we fix the path in the init script

* Use "echo -n" for the Starting and Stopping messages, so that the OK / FAIL stay on the same line.

* There is no need to pass the inadyn binary path during the stop sequence, as long as we pass the appropriate -p option with the path to the PID file created by inadyn, so we do this.

* Pass the -q option to the start sequence, since it's passed in the stop sequence.

* Fix the configuration file to use an existing dyndns_system and avoid a failure at startup.

---
 package/inadyn/S70inadyn   | 10 +++++-----
 package/inadyn/inadyn.conf |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)
package/inadyn/S70inadyn +5 -5
index b20048c..b2aaa0a 100644
@@ -14,15 +14,15 @@ VR_INADYN=/var/run/inadyn
 
 case "$1" in
 	start)
-		echo "Starting inadyn: "
-		start-stop-daemon -S -x /usr/bin/inadyn
+		echo -n "Starting inadyn: "
+		start-stop-daemon -q -S -p ${VR_INADYN}/inadyn.pid -x /usr/sbin/inadyn
 		[ $? = 0 ] && echo "OK" || echo "FAIL"
 		;;
 	stop)
-		echo  "Stopping inadyn: "
-		start-stop-daemon -q -K -x /usr/bin/inadyn
+		echo -n "Stopping inadyn: "
+		start-stop-daemon -q -K -p ${VR_INADYN}/inadyn.pid
 		[ $? = 0 ] && echo "OK" || echo "FAIL"
-		rm -f /var/run/inadyn/inadyn.pid
+		rm -f ${VR_INADYN}/inadyn.pid
 		;;
 	restart)
 		"$0" stop
package/inadyn/inadyn.conf +1 -1
index b5877f7..93ad0b5 100644
@@ -5,7 +5,7 @@ background
 update_period_sec 600 # Check for a new IP every 600 seconds
 username test		# replace 'test' with your username
 password test		# replace 'test' with your password
-dyndns_system dyndns at dyndns.org   # replace w/ your provider
+dyndns_system default at dyndns.org   # replace w/ your provider
 
 #  uncomment the alias statement below to test it on your system
 alias test.homeip.net
Signature · 1 line
2.4.5
#1 lore raw

PATCH 1/1 inadyn: fix init script and default config file

Quoted 6 lines · Thomas Petazzoni

This commit does a number of fixes to the inadyn package to make it work properly "out of the box":

* inadyn is installed in /usr/sbin, not /usr/bin, so we fix the path in the init script

Quoted 6 lines

* Use "echo -n" for the Starting and Stopping messages, so that the OK / FAIL stay on the same line.

* There is no need to pass the inadyn binary path during the stop sequence, as long as we pass the appropriate -p option with the path to the PID file created by inadyn, so we do this.

Actually there is a reason to pass it: if inadyn has already died and another
process has taken over the PID, then start-stop-daemon can check that both the PID and the process executable match. So keep it (if it works).

With that fixed:

Regards,
Arnout

Quoted 53 lines

* Pass the -q option to the start sequence, since it's passed in the stop sequence.

* Fix the configuration file to use an existing dyndns_system and avoid a failure at startup.

---
 package/inadyn/S70inadyn   | 10 +++++-----
 package/inadyn/inadyn.conf |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)
package/inadyn/S70inadyn +5 -5
index b20048c..b2aaa0a 100644
@@ -14,15 +14,15 @@ VR_INADYN=/var/run/inadyn
 
 case "$1" in
 	start)
-		echo "Starting inadyn: "
-		start-stop-daemon -S -x /usr/bin/inadyn
+		echo -n "Starting inadyn: "
+		start-stop-daemon -q -S -p ${VR_INADYN}/inadyn.pid -x /usr/sbin/inadyn
 		[ $? = 0 ] && echo "OK" || echo "FAIL"
 		;;
 	stop)
-		echo  "Stopping inadyn: "
-		start-stop-daemon -q -K -x /usr/bin/inadyn
+		echo -n "Stopping inadyn: "
+		start-stop-daemon -q -K -p ${VR_INADYN}/inadyn.pid
 		[ $? = 0 ] && echo "OK" || echo "FAIL"
-		rm -f /var/run/inadyn/inadyn.pid
+		rm -f ${VR_INADYN}/inadyn.pid
 		;;
 	restart)
 		"$0" stop
package/inadyn/inadyn.conf +1 -1
index b5877f7..93ad0b5 100644
@@ -5,7 +5,7 @@ background
 update_period_sec 600 # Check for a new IP every 600 seconds
 username test		# replace 'test' with your username
 password test		# replace 'test' with your password
-dyndns_system dyndns at dyndns.org   # replace w/ your provider
+dyndns_system default at dyndns.org   # replace w/ your provider
 
 #  uncomment the alias statement below to test it on your system
 alias test.homeip.net
Signature · 6 lines
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
#2 lore raw

PATCH 1/1 inadyn: fix init script and default config file

In reply to Thomas Petazzoni
Quoted 2 lines · Thomas Petazzoni

* Fix the configuration file to use an existing dyndns_system and avoid a failure at startup.

You definitely don't want this, if it's not configured and requires configuration for any sane usage let it be so.
You don't want to do this kind of thing by default:
https://en.wikipedia.org/wiki/NTP_server_misuse_and_abuse

Also trusting that 'background' is in the config doesn't seem foolproof either, dropping it and adding -b to the initscript sounds better to me.

Regards.

#3 lore raw

PATCH 1/1 inadyn: fix init script and default config file

Gustavo,

Quoted 11 lines · Gustavo Zacarias
Quoted 2 lines · Thomas Petazzoni

* Fix the configuration file to use an existing dyndns_system and avoid a failure at startup.

You definitely don't want this, if it's not configured and requires configuration for any sane usage let it be so.
You don't want to do this kind of thing by default:
https://en.wikipedia.org/wiki/NTP_server_misuse_and_abuse

Hu? What's the relation between NTP server misuse and Inadyn which is a tool that updates a DynDNS entry with your IP address? Am I missing something?

Quoted 2 lines

Also trusting that 'background' is in the config doesn't seem foolproof either, dropping it and adding -b to the initscript sounds better to me.

But then if 'background' is in the config file, we will background it twice. Since inadyn generates its own PID file, it might be OK, but it's not that nice either.

I think our init scripts are anyway done to work fine with our default configuration files. If you change the configuration file, you have to be ready to adjust init scripts as well I'd say, no?

Thanks,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#4 lore raw

PATCH 1/1 inadyn: fix init script and default config file

Quoted 8 lines · Thomas Petazzoni
Quoted 4 lines

You definitely don't want this, if it's not configured and requires configuration for any sane usage let it be so.
You don't want to do this kind of thing by default:
https://en.wikipedia.org/wiki/NTP_server_misuse_and_abuse

Hu? What's the relation between NTP server misuse and Inadyn which is a tool that updates a DynDNS entry with your IP address? Am I missing something?

If you ship a config that starts up inadyn with user=test pass=test with some X dyndns service that could potentially flood said service/server with a bogus/useless setting.
It might not matter with small usage scenarios, but if a firmware image is shipped for some networking appliance which doesn't sanitize the config then you've got a similar scenario to the NTP flood.

Quoted 7 lines

But then if 'background' is in the config file, we will background it twice. Since inadyn generates its own PID file, it might be OK, but it's not that nice either.

I think our init scripts are anyway done to work fine with our default configuration files. If you change the configuration file, you have to be ready to adjust init scripts as well I'd say, no?

I haven't tested the detail, but -b is probably the same as "background" in the config and won't background twice (i'm talking of the inadyn -b option, not backgrounding from start-stop-daemon).
Regards.

#5 lore raw

PATCH 1/1 inadyn: fix init script and default config file

Dear Gustavo Zacarias,

Quoted 6 lines · Gustavo Zacarias

If you ship a config that starts up inadyn with user=test pass=test with some X dyndns service that could potentially flood said service/server with a bogus/useless setting.
It might not matter with small usage scenarios, but if a firmware image is shipped for some networking appliance which doesn't sanitize the config then you've got a similar scenario to the NTP flood.

Ah, yes, I see. So maybe we should instead have:

ENABLED="no"

in /etc/default/inadyn

and in the SXXinadyn, if ENABLED=="no", do something like:

Starting inadyn: SKIPPED

or something like that.

Thoughts?

Quoted 11 lines
Quoted 7 lines

But then if 'background' is in the config file, we will background it twice. Since inadyn generates its own PID file, it might be OK, but it's not that nice either.

I think our init scripts are anyway done to work fine with our default configuration files. If you change the configuration file, you have to be ready to adjust init scripts as well I'd say, no?

I haven't tested the detail, but -b is probably the same as "background" in the config and won't background twice (i'm talking of the inadyn -b option, not backgrounding from start-stop-daemon).

Ah, you're talking about inadyn -b option, while I was thinking of the start-stop-daemon option. Then it definitely makes sense, and I can get rid of the background option in the example config file.

Sounds OK?

Thanks!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#6 lore raw

PATCH 1/1 inadyn: fix init script and default config file

Quoted 6 lines · Thomas Petazzoni

Ah, yes, I see. So maybe we should instead have:

ENABLED="no"

in /etc/default/inadyn

Quoted 7 lines

and in the SXXinadyn, if ENABLED=="no", do something like:

Starting inadyn: SKIPPED

or something like that.

Thoughts?

Yes, the initscripts enable/disable logic is pending (my fault probably, though i didn't follow the conclusions with detail).

Quoted 5 lines

Ah, you're talking about inadyn -b option, while I was thinking of the start-stop-daemon option. Then it definitely makes sense, and I can get rid of the background option in the example config file.

Sounds OK?

Yup, it's mostly covering for user omission since it costs nothing. With the addition of -- -b :)
It also makes the systemd service more resilient, since it normally doesn't expect a background (at least via BR default config). Regards.