Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 11 Apr 2012 18:51:00 -0700 (PDT)
From:      Doug Barton <dougb@FreeBSD.org>
To:        Stanislav Sedov <stas@FreeBSD.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r234093 - in head/etc: defaults rc.d
Message-ID:  <alpine.BSF.2.00.1204111827441.20567@bcgv.qbhto.arg>
In-Reply-To: <201204100927.q3A9Rf3r013400@svn.freebsd.org>
References:  <201204100927.q3A9Rf3r013400@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--0-318390063-1334195461=:20567
Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed

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/

--0-318390063-1334195461=:20567
Content-Type: TEXT/PLAIN; charset=US-ASCII; name=kfd-rcd.diff
Content-Transfer-Encoding: BASE64
Content-ID: <alpine.BSF.2.00.1204111851000.20567@bcgv.qbhto.arg>
Content-Description: 
Content-Disposition: attachment; filename=kfd-rcd.diff

SW5kZXg6IGRlZmF1bHRzL3JjLmNvbmYNCj09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT0NCi0tLSBkZWZhdWx0cy9yYy5jb25mCShyZXZpc2lvbiAyMzQxNjQpDQor
KysgZGVmYXVsdHMvcmMuY29uZgkod29ya2luZyBjb3B5KQ0KQEAgLTI5Nyw4
ICsyOTcsOSBAQA0KIGthZG1pbmQ1X3NlcnZlcj0iL3Vzci9saWJleGVjL2th
ZG1pbmQiCSMgcGF0aCB0byBrZXJiZXJvcyA1IGFkbWluIGRhZW1vbg0KIGtw
YXNzd2RkX3NlcnZlcl9lbmFibGU9Ik5PIgkjIFJ1biBrcGFzc3dkZCAob3Ig
Tk8pDQoga3Bhc3N3ZGRfc2VydmVyPSIvdXNyL2xpYmV4ZWMva3Bhc3N3ZGQi
CSMgcGF0aCB0byBrZXJiZXJvcyA1IHBhc3N3ZCBkYWVtb24NCi1rZmRfc2Vy
dmVyX2VuYWJsZT0iTk8iCQkjIFJ1biBrZmQgKG9yIE5PKQ0KLWtmZF9zZXJ2
ZXI9Ii91c3IvbGliZXhlYy9rZmQiCSMgcGF0aCB0byBrZXJiZXJvcyA1IGtm
ZCBkYWVtb24NCitrZmRfZW5hYmxlPSJOTyIgCQkjIFJ1biBrZmQgKG9yIE5P
KQ0KK2tmZF9wcm9ncmFtPSIvdXNyL2xpYmV4ZWMva2ZkIgkjIFBhdGggdG8g
a2VyYmVyb3MgNSBrZmQgZGFlbW9uDQora2ZkX2ZsYWdzPSItaSINCiANCiBn
c3NkX2VuYWJsZT0iTk8iCQkjIFJ1biB0aGUgZ3NzZCBkYWVtb24gKG9yIE5P
KS4NCiBnc3NkX2ZsYWdzPSIiCQkJIyBGbGFncyBmb3IgZ3NzZC4NCkluZGV4
OiByYy5kL2tmZA0KPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQ0KLS0tIHJjLmQv
a2ZkCShyZXZpc2lvbiAyMzQxNjQpDQorKysgcmMuZC9rZmQJKHdvcmtpbmcg
Y29weSkNCkBAIC0xLDE5ICsxLDIwIEBADQogIyEvYmluL3NoDQotIw0KKw0K
ICMgJEZyZWVCU0QkDQogIw0KLQ0KICMgUFJPVklERToga2ZkDQotIyBCRUZP
UkU6IERBRU1PTg0KKyMgUkVRVUlSRToga2VyYmVyb3MNCisjIEtFWVdPUkQ6
IHNodXRkb3duDQogDQogLiAvZXRjL3JjLnN1YnINCiANCi1uYW1lPSJrZmQi
DQorbmFtZT1rZmQNCityY3Zhcj1rZmRfZW5hYmxlDQorDQogbG9hZF9yY19j
b25maWcgJG5hbWUNCi1yY3Zhcj0ia2ZkX3NlcnZlcl9lbmFibGUiDQotdW5z
ZXQgc3RhcnRfY21kDQotY29tbWFuZD0iJHtrZmRfc2VydmVyfSINCi1rZmRf
ZmxhZ3M9Ii1pIg0KKw0KKyMgdW5zZXQgc3RhcnRfY21kID8/Pw0KKw0KIGNv
bW1hbmRfYXJncz0iJiINCiANCiBydW5fcmNfY29tbWFuZCAiJDEiDQpJbmRl
eDogcmMuZC9EQUVNT04NCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0NCi0tLSBy
Yy5kL0RBRU1PTgkocmV2aXNpb24gMjM0MTY0KQ0KKysrIHJjLmQvREFFTU9O
CSh3b3JraW5nIGNvcHkpDQpAQCAtNCw3ICs0LDcgQEANCiAjDQogDQogIyBQ
Uk9WSURFOiBEQUVNT04NCi0jIFJFUVVJUkU6IE5FVFdPUktJTkcgU0VSVkVS
Uw0KKyMgUkVRVUlSRTogTkVUV09SS0lORyBTRVJWRVJTIGtmZA0KIA0KICMJ
VGhpcyBpcyBhIGR1bW15IGRlcGVuZGVuY3ksIHRvIGVuc3VyZSB0aGF0IGdl
bmVyYWwgcHVycG9zZSBkYWVtb25zDQogIwlhcmUgcnVuIF9hZnRlcl8gdGhl
IGFib3ZlIGFyZS4NCg==

--0-318390063-1334195461=:20567--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?alpine.BSF.2.00.1204111827441.20567>