cvs commit: ports/security/vpnc Makefile
ports/security/vpnc/files vpnc.in vpnc.sh
gabor at FreeBSD.org
Wed Feb 28 18:27:45 UTC 2007
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
>> - 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)
> 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
> 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 $?
> # 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
> 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
> echo "Running 'vpnc $current $vpnc_flags' failed."
> return $status
> # 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.
> return $?
> # A record of vpnc connections is present. Attempt a
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?
More information about the cvs-all