On Wed, Apr 22, 2009 at 03:30:48PM -0400, Brian Utterback wrote:
> http://cr.opensolaris.org/~blu/ntpv4/sfwnv-webrev/
- $SRC/cmd/ntpd/Makefile:41
41 CONFIGURE_OPTIONS += --with-openssl-libdir=/usr/sfw/lib
a) OpenSSL has moved out of /usr/sfw and into /usr. Don't forget to
fix LD_OPTIONS too...
b) You didn't mention OpenSSL in the ARC case. I think you still
need to sign a contract in order to use it.
- $SRC/cmd/ntpd/Makefile:43
43 CONFIGURE_OPTIONS += --enable-debugging
What does --enable-debugging do?
- Just out of curiosity, what configure options are you NOT enabling?
- I never knew about .patched/PATCHES thing. Cool.
What's not cool, IMO, is that it doesn't use unified diffs.
- $SRC/cmd/ntpd/Patches/etcfix.patch
Does /var/ntp need to be documented?
- $SRC/cmd/ntpd/Patches/noextra.patch
Could you patch configure.ac and run autoconf instead?
Why remove ntpsnmpd when there's already a ./configure
--without-ntpsnmpd option (and you're using it)?
What about the others? Are there --without-* options for them?
Or is this about build times (looking at the fact that you're having
to modify SUBDIRS I'm guessing that's what this is about)?
- Could the mDNS registration/unregistration not be done entirely in
the SMF start/stop methods?
- $SRC/cmd/ntpd/Patches/ntpwait.patch
Is there a need to update the comment at 23?
- $SRC/cmd/ntpd/Solaris/ntp.server
It'd be nice if we had a page where people could report what time
source hardware works for them on Solaris...
References to VME... wow! :)
- Could the start method use ppriv(1) to run ntpd with even fewer
privs? I understand why the start method needs proc_fork,proc_exec,
but surely ntpd does not (right?).
- $SRC/pkgdefs/SUNWntpu/depend
What about a dependency on SUNWopenssl?
- $SRC/cmd/ntpd/Solaris/ntp.xml:61
61 timeout_seconds='1800'>
1800 seconds?!!
- $SRC/cmd/ntpd/Solaris/ntp:52-53
52 # If this is a start then set the panic gate. Not if it is a refresh.
53 [ "$1" = "start" ] && args="-g"
But ntp.xml defines a restart method using the same script as the
start method. Which is it, a restart or a refresh?
- $SRC/cmd/ntpd/Solaris/ntp
Any chance this could be re-written to be a ksh script?
- $SRC/cmd/ntpd/Solaris/ntp
Values produced by svcprop need some validation/checking. Consider
these lines:
87 # Set up debugging.
88 deb=`svcprop -c -p config/debuglevel $SMF_FMRI`
89 debfile=`svcprop -c -p config/debugfile $SMF_FMRI`
...
94 /usr/lib/inet/ntpd $args --set-debug-level=$deb >$debfile &
What if $debfile is set to something nasty? Privilege escalation?
Nico
--
