David Powell wrote: > 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. >
Yes, error handling can be improved. I'll make the change after getting your review comments. >>> 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. > I'm on it. >>> 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 I'll make the change after getting your review comments. -tony _______________________________________________ networking-discuss mailing list [email protected]
