cvs commit: ports/security/vpnc Makefile
ports/security/vpnc/files vpnc.in vpnc.sh
Doug Barton
dougb at FreeBSD.org
Mon Feb 26 23:41:24 UTC 2007
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 <lon_kamikaze at gmx.de> (with fixes from maintainer)
> Approved by: Christian Lackas <delta at lackas.net> (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
...
(BTW, there is a type at "asume")
hope this helps,
Doug
--
This .signature sanitized for your protection
More information about the cvs-ports
mailing list