On Wed, Nov 17, 2010 at 7:40 PM, Gabe Black <[email protected]> wrote:
> > > On November 17th, 2010, 6:24 p.m., *Steve Reinhardt* wrote: > > > src/python/m5/params.py<http://reviews.m5sim.org/r/316/diff/1/?file=5207#file5207line721> > (Diff > revision 1) > > def swig_predecls(cls, code): > > 721 > > def __init__(self, *args, **kwargs): > > All this kwargs stuff seems like overkill, and I'm not sure all the corner > cases are handled sensibly (passing in args[0] as elseVal for both ip and > netmask seems particularly suspect), but now that it's written I guess > there's no point in deleting it. A comment that lists all the options for > specifying a value would be helpful though. > > This is based off of the implementation of the Range param type, and lets > you set the ip and port or ip and netmask explicitly. That's very handy when > generating those values programmatically which is exactly how I've been using > them. You're right that not all cases are covered. If kargs has neither ip > nor netmask and one value args, that will end up in both ip and netmask. I'll > look it over and see if some of the gaps can be covered easily. I was > mimicking the Range type like I mentioned, but maybe that's not the best > approach. > > I figured you were modeling it after Range... not that it's bad, really, just more than is needed, and maybe more than is really useful. If it were me, I'd just do the string, though I can see where the keyword and/or positional versions could be handy, and providing both is generous. However the ability to have one positional and one keyword argument seems like you're just asking people to write their scripts cryptically. > > On November 17th, 2010, 6:24 p.m., *Steve Reinhardt* wrote: > > > src/python/m5/util/convert.py<http://reviews.m5sim.org/r/316/diff/1/?file=5208#file5208line261> > (Diff > revision 1) > > def toIpAddress(value): > > 261 > > if not 0 <= int(byte) <= 256: > > Shouldn't this be "< 256"? (and "< 32" below too?) Didn't you just point > out this same bug in some existing code here? > > < 256 yes, < 32 no. The max byte value is 255, but the netmask can have 0 to > 32 bits set I think, where 0 would be all possible IPs and 32 would be one > specific IP. Let me know if that's not true. > > Good point, that is correct about the netmask. Steve
_______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
