Doug Barton schrieb:
Gabor Kovesdan wrote:
gabor 2007-02-26 18:57:31 UTC
FreeBSD ports repository
Modified files:
security/vpnc Makefile
Added files:
security/vpnc/files vpnc.in
Log:
- Update rc.d script
- Use USE_RC_SUBR instead of direct patching
- Bump PORTREVISION
PR: ports/107675 http://www.FreeBSD.org/cgi/query-pr.cgi?pr=107675
Submitted by: Dominic Fandrey <[EMAIL PROTECTED]> (with fixes from
maintainer)
Approved by: Christian Lackas <[EMAIL PROTECTED]> (maintainer),
erwin (mentor, implicit)
Revision Changes Path
1.21 +4 -6 ports/security/vpnc/Makefile
1.1 +95 -0 ports/security/vpnc/files/vpnc.in (new)
http://www.FreeBSD.org/cgi/cvsweb.cgi/ports/security/vpnc/Makefile.diff?&r1=1.20&r2=1.21&f=h
http://www.FreeBSD.org/cgi/cvsweb.cgi/ports/security/vpnc/files/vpnc.in
First off I'd like to say that I always appreciate when people convert
their ports to the new rc.d framework, so please don't take any of
these comments as critical. They are meant to be constructive,
especially given that there are so many people involved in this one
commit. :)
The Makefile changes look good, but there are a few problems with the
new rc.d script. If you have any questions don't hesitate to ask for
clarification.
1. Please remove the FreeBSD KEYWORD. We have ignored that keyword for
a long time now, and yet somehow it keeps creeping back in. You might
also consider changing the REQUIRE to LOGIN, unless there is a
compelling reason to have it where it is.
2. In your default variable assignments, it's not necessary (and
probably not a good idea) to add empty assignments for _conf and
_flags. Rather you should document those flags in the comments as you
do the _conf variable.
3. In vpnc_start(), you don't want to use a bare 'if [ "$variable" ]'.
That's not good style, and it opens you up to problems. I would write
that section like this:
if [ -z "$vpnc_conf" ]; then
# No configuration files given, run unmanaged.
$command $vpnc_flags
return $?
fi
# A list of configurations is present. Connect managing
....
4. In both functions, you use the following style:
for foo in $bar; (
...
)
It's probably better to use:
for foo in $bar; do
...
done
That way you avoid the subshell, and any possible related problems.
5. In vpnc_start() you could simplify the code by doing:
if ! $command $current $vpnc_flags; then
status=$?
echo "Running 'vpnc $current $vpnc_flags' failed."
return $status
fi
# Move files to allow a clean shutdown
...
6. For simplicity, I'd make the same change in vpnc_stop() that I
suggested in _start, namely to do:
if [ ! -e "$vpnc_record" ]; then
/bin/sleep 1
# There's no record of connections, assume unmanaged shutdown.
$command-disconnect
return $?
fi
# A record of vpnc connections is present. Attempt a
...
Hello,
thanks for pointing these out. Could you check the patch at
http://gabor.t-hosting.hu/patches/security-vpnc.diff to make sure I got
all your points correctly? While here, I changed the wrapping a bit to
make it easier to read.
Christian, do you approve these changes suggested by Doug?
Regards,
Gabor
_______________________________________________
cvs-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "[EMAIL PROTECTED]"
