svn commit: r234093 - in head/etc: defaults rc.d

[Doug Barton]

This script has many problems. First, new scripts in the base should 
never use BEFORE since this makes debugging rcorder issues harder. 
Please instead add this to the REQUIRE line in DAEMON. You can use 
'service -r' to get an idea of where it will be included at boot time.

Second, for new scripts the name of the file, $name, and PROVIDE should 
all the the same, in this case "kfd". That implies that the rcvar should 
be kfd_enable unless there is a very good reason for it to be 
different, which in this case there is not one that I can see. (FYI, 
the "d" in "kfd" implies "server.")

Scripts that start persistent services should always include "KEYWORD: 
shutdown" so that they get started cleanly. It's not clear to me if this 
script should also include the nojail keyword. It's also generally a 
good idea to add a REQUIRE line unless it truly doesn't matter. I 
guessed at REQUIRE: kerberos ala the kadamind script, if that's wrong 
please let me know.

Next, the arguments in the script, and the script generally, don't
follow the default format. It's very helpful when doing mass 
reviews/updates if the script looks the same as other similar scripts 
unless there is a good reason for it to be different. The reference at 
http://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/rc-scripts.html 
should help.

It's also unclear why you are unset'ing start_cmd, but not supplying 
your own start method. The way that you're using kfd_server is also 
totally wrong, as is hard-coding kfd_flags in the script. If the -i 
argument is always needed, it should be included in command_args. If 
it's not always needed, but is generally recommended, you should include 
it in /etc/defaults/rc.conf so that the user can easily override it. 
I've assumed the latter, if it should always be included please let me 
know.

Finally, you neglected to update rc.conf.5.

The attached patch fixes the problems mentioned above, modulo the 
rc.conf.5 update which I'll leave to you. In order to avoid 
inconvenience to those tracking HEAD who want to use this new feature I 
plan to commit this in the next couple of days if I don't hear from you.

In the future it would probably be a good idea to submit patches to 
freebsd-rc@ for review.

hth,

Doug


On Tue, 10 Apr 2012, Stanislav Sedov wrote:

> Author: stas
> Date: Tue Apr 10 09:27:41 2012
> New Revision: 234093
> URL: http://svn.freebsd.org/changeset/base/234093
>
> Log:
>  - Add rc.d script for kfd, kerberos forwarded tickets daemon.
>
> Added:
>  head/etc/rc.d/kfd   (contents, props changed)
> Modified:
>  head/etc/defaults/rc.conf
>  head/etc/rc.d/Makefile
>
> Modified: head/etc/defaults/rc.conf
> ==============================================================================
> --- head/etc/defaults/rc.conf	Tue Apr 10 07:38:58 2012	(r234092)
> +++ head/etc/defaults/rc.conf	Tue Apr 10 09:27:41 2012	(r234093)
> @@ -297,6 +297,8 @@ kadmind5_server_enable="NO"	# Run kadmin
> kadmind5_server="/usr/libexec/kadmind"	# path to kerberos 5 admin daemon
> kpasswdd_server_enable="NO"	# Run kpasswdd (or NO)
> kpasswdd_server="/usr/libexec/kpasswdd"	# path to kerberos 5 passwd daemon
> +kfd_server_enable="NO"		# Run kfd (or NO)
> +kfd_server="/usr/libexec/kfd"	# path to kerberos 5 kfd daemon
>
> gssd_enable="NO"		# Run the gssd daemon (or NO).
> gssd_flags=""			# Flags for gssd.
>
> Modified: head/etc/rc.d/Makefile
> ==============================================================================
> --- head/etc/rc.d/Makefile	Tue Apr 10 07:38:58 2012	(r234092)
> +++ head/etc/rc.d/Makefile	Tue Apr 10 09:27:41 2012	(r234093)
> @@ -66,6 +66,7 @@ FILES=	DAEMON \
> 	kadmind \
> 	kerberos \
> 	keyserv \
> +	kfd \
> 	kld \
> 	kldxref \
> 	kpasswdd \
>
> Added: head/etc/rc.d/kfd
> ==============================================================================
> --- /dev/null	00:00:00 1970	(empty, because file is newly added)
> +++ head/etc/rc.d/kfd	Tue Apr 10 09:27:41 2012	(r234093)
> @@ -0,0 +1,19 @@
> +#!/bin/sh
> +#
> +# $FreeBSD$
> +#
> +
> +# PROVIDE: kfd
> +# BEFORE: DAEMON
> +
> +. /etc/rc.subr
> +
> +name="kfd"
> +load_rc_config $name
> +rcvar="kfd_server_enable"
> +unset start_cmd
> +command="${kfd_server}"
> +kfd_flags="-i"
> +command_args="&"
> +
> +run_rc_command "$1"
>
>


-- 

 	It's always a long day; 86400 doesn't fit into a short.

 	Breadth of IT experience, and depth of knowledge in the DNS.
 	Yours for the right price.  :)  http://SupersetSolutions.com/
-------------- next part --------------
Index: defaults/rc.conf
===================================================================
--- defaults/rc.conf	(revision 234164)
+++ defaults/rc.conf	(working copy)
@@ -297,8 +297,9 @@
 kadmind5_server="/usr/libexec/kadmind"	# path to kerberos 5 admin daemon
 kpasswdd_server_enable="NO"	# Run kpasswdd (or NO)
 kpasswdd_server="/usr/libexec/kpasswdd"	# path to kerberos 5 passwd daemon
-kfd_server_enable="NO"		# Run kfd (or NO)
-kfd_server="/usr/libexec/kfd"	# path to kerberos 5 kfd daemon
+kfd_enable="NO" 		# Run kfd (or NO)
+kfd_program="/usr/libexec/kfd"	# Path to kerberos 5 kfd daemon
+kfd_flags="-i"
 
 gssd_enable="NO"		# Run the gssd daemon (or NO).
 gssd_flags=""			# Flags for gssd.
Index: rc.d/kfd
===================================================================
--- rc.d/kfd	(revision 234164)
+++ rc.d/kfd	(working copy)
@@ -1,19 +1,20 @@
 #!/bin/sh
-#
+
 # $FreeBSD$
 #
-
 # PROVIDE: kfd
-# BEFORE: DAEMON
+# REQUIRE: kerberos
+# KEYWORD: shutdown
 
 . /etc/rc.subr
 
-name="kfd"
+name=kfd
+rcvar=kfd_enable
+
 load_rc_config $name
-rcvar="kfd_server_enable"
-unset start_cmd
-command="${kfd_server}"
-kfd_flags="-i"
+
+# unset start_cmd ???
+
 command_args="&"
 
 run_rc_command "$1"
Index: rc.d/DAEMON
===================================================================
--- rc.d/DAEMON	(revision 234164)
+++ rc.d/DAEMON	(working copy)
@@ -4,7 +4,7 @@
 #
 
 # PROVIDE: DAEMON
-# REQUIRE: NETWORKING SERVERS
+# REQUIRE: NETWORKING SERVERS kfd
 
 #	This is a dummy dependency, to ensure that general purpose daemons
 #	are run _after_ the above are.