Wyllys Ingersoll writes: > I need a code reviewer for the putback of The onion Router code into > the SFW consolidation. Pretty please. > > http://cr.opensolaris.org/~wyllys/tor
I looked at all of the files (except for the .tar.gz file), so if I didn't mention it below, I reviewed it, and it looked ok to me. usr/src/cmd/tor/tor.sh 58,70: I prefer to see "/^$port/" and "/^$addr/" in the awk code than to see grep+awk. Grep seems redundant here. 83: are other forms legal in the file? Does "0.0.0" or "0.0" or "0" parse? What about "::0"? 44-46,131-133: this seems unnecessary ... why not just ":kill" in the manifest? usr/src/cmd/tor/install-sfw 63: uuoc usr/src/cmd/tor/Makefile.sfw 24: don't forget to "sccs create". 53: "env -"? Why is that needed? 68: this is a little odd. It seems to depend on "configure" being extracted with an old time/date stamp. I think it'd be a little more obvious as "$(PROD)/configure: FRC" ... but this is just a nit. usr/src/pkgdefs/Makefile 168: nit: seems to be missing a merge. usr/src/pkgdefs/SUNWtor-root/depend Is there a common 'depend' file that could be used instead? There seems to be nothing unique here. It also seems to be generated by the Makefile. Should this file be in the workspace at all? 2: really? usr/src/pkgdefs/SUNWtor-root/prototype_com 24: nit: 'pragma' makes no sense here. usr/src/pkgdefs/SUNWtor-root/pkginfo.tmpl 45: 'preserve' seems to be unused. 54: nit: shell scripts are ordinarily delivered without the '.sh' extension. -- James Carlson, Solaris Networking <james.d.carlson at sun.com> Sun Microsystems / 35 Network Drive 71.232W Vox +1 781 442 2084 MS UBUR02-212 / Burlington MA 01803-2757 42.496N Fax +1 781 442 1677
