Tony Nguyen wrote: > Dave, > > See my responses inline and the updated webrev at: > > http://cr.opensolaris.org/~tonyn/firewall13Jan2009-inc/ > http://cr.opensolaris.org/~tonyn/firewall13Jan2009/
Just responding to your mail; I'm still reviewing the code: >> servinfo.c`uaddr2port: >> >> I believe the universal address you get from rpcb_getmaps is >> provided by the server. Though the server should be our rpcbind, I >> don't think it's safe to assume the format of the string, >> especially since universal addresses aren't limited to the formats >> you list. This function needs to extract port numbers in a more >> generic, robust way. > > Yes, the server is rpcbind but universal address doesn't have > standardized formats at the moment. There is a draft rfc at > > http://www.ietf.org/internet-drafts/draft-ietf-nfsv4-rpc-netid-05.txt > > Since universal addresses have the form: > > h1.h2.h3.h4.p1.p2 > x1:x2:x3:x4:x5:x6:x7:x8.p1.p2 > ::1.p1.p2 > > How about getting the last two octets? Do you see an alternative? That's what I had in mind. Your code should handle any universal address, and should fail gracefully if what you're given isn't a valid universal address. >> ipfilter.xml: >> >> Some of your <description>s are too verbose. They should only a >> sentence or two, and contain no line breaks. The pre-formatted >> content you currently have will be mangled by consumers and should >> be removed. e.g. for the 'apply_to' and 'exceptions' properties I >> would say something like: >> >> The host and network IPs, network interfaces, and ippools to deny >> if the policy is set to deny, or accept if the policy is set to >> accept. > > I thought examples would make it easier for users to set property > values. If the content will be mangled, then users will need to use the > manpage. I'll make the suggested change. > >> You need to have <common_name>s in addition to <description>s. > > common_names for properties are added. I didn't add common_names for > each property values as they don't seem necessary. In general, anything that has a description should have a common_name. I suppose in this case the property value itself is the common name, at least in the C locale. If the recommended practice for template consumers is to fall back on the property value if a localized value name doesn't exist, and the instructions for the l10n of services specify that a common_name should be provided even when one isn't provided in the C locale, then not providing a common_name should be all right. I'd check with the templates experts. >> process_service: >> >> `something > /dev/null` is a strange construct; I'm not sure what >> you're trying to do, but I know that it will cause some versions of >> ksh93 to crash. > > I'm trying to exec another script in a subshell but don't want the > output. Would > > ( exec $cmd $args >/dev/null ) > > be better? If all you are trying to do is run a script, you should be able to just do $cmd $args > /dev/null I think your problem is that $cmd contains both the commands and arguments, and requires expansion before being run. For that I would eval $cmd $args > /dev/null Dave
