On Fri, 2012-05-04 at 23:29 -0600, Philip Prindeville wrote: > I'd combine all of these as: > > local device server username password ...
Hm. I was just trying to copy local style, which in ppp.sh had each one
with its usage. Which makes slightly more sense to me; it makes it
simpler to add and remove options.
> > + [ "$peerdns" -eq 1 ] && {
> > + peerdns="usepeerdns"
> > + } || {
> > + peerdns=""
> > + add_dns "$config" $dns
> > + }
>
> Not clear how this is better than:
>
> if [ "$peerdns" -eq 1 ]; then
> peerdns="userpeerdns"
> else
> peerdns=""
> add_dns "$config" $dns
> fi
Not sure, but again that's what's in ppp.sh. Perhaps something to do
with shell compatibility, but I don't know which shells couldn't cope
with your version (which is how I'd write it myself).
> I'd have done:
>
> cat <<__EOF__ >> "${optfile}"
> ${keepalive:+lcp-echo-interval $interval lcp-echo-failure ${keepalive%%[, ]*}}
> ...
> __EOF__
That was my first thought too. I can't remember now why I didn't do it
that way. Perhaps I thought that some of those individual echo commands
might be conditional. But yeah, cat <<EOF would be cleaner.
>
> > +
> > + xl2tpd-control remove l2tp-${config}
> > + # Wait and ensure pppd has died.
> > + while [ -d /sys/class/net/l2tp-${config} ]; do
> > + sleep 1
> > + done
>
> If there's a bug, this might never exit... Maybe add a counter?
Makes sense.
> Use of quotes around ${optfile} is inconsistent.
Hm, true. I think they're fairly pointless quotes, but I should at least
be consistent about it.
> }
>
> I'd try to match the current indent style.
This is a patch from elsewhere; not a local one. It's from the Fedora
package, and also been pushed upstream I believe. So I'd rather not mess
with it (except to remove the modprobe invocation).
--
dwmw2
smime.p7s
Description: S/MIME cryptographic signature
_______________________________________________ openwrt-devel mailing list [email protected] https://lists.openwrt.org/mailman/listinfo/openwrt-devel
