Looks good to me now - much clearer ;) Thanks,
Darren. On 01/09/2011 22:14, Harold Shaw wrote: > I have made the changes discussed below and reposted the webrev: > > https://cr.opensolaris.org/action/browse/caiman/hshaw/7082285/webrev/ > > Thanks, > Harold > > On 09/01/11 02:24, Darren Kenny wrote: >> Hi Harold, >> >> It took me a few goes to understand what exactly this code was doing - which >> makes me think that it needs some comments to explain it a little more. >> >> Looking at the code, it looks like there is a possible hole in the logic if >> somehow you got addresses like: >> >> 192.168.1.1 >> 192.168.1.2 >> 192.168.2.3<- Note the subnet also changes. >> >> This wouldn't see this as a new range, but would think it's part of the >> currently processed range since the expression: >> >> int(client_ip_tuple) != int(last_ip_tuple) + 1: >> >> would evaluate to False. >> >> Is this possible to occur in reality? > I believe that it can. I have made the changes necessary to account for > this. >> If so, maybe you need two groups in the RE - one for the subnet and one for >> the >> last digit (why do you call this a tuple - it's confusing I think) - and then >> compare the subnets too in the expression at line 164. > I have refactored the code and renamed the variables so that it is, > hopefully more understandable. Let me know if it addresses your concerns. >> Thanks, >> >> Darren. >> >> On 31/08/2011 19:16, Harold Shaw wrote: >>> Can I get another reviewer for this? >>> >>> Thanks, >>> Harold >>> >>> On 08/31/11 08:33, Harold Shaw wrote: >>>> Thanks, William. >>>> >>>> Harold >>>> >>>> On 08/31/11 07:08, William Schumann wrote: >>>>> Harold, >>>>> This fix looks fine to me. >>>>> William >>>>> >>>>> On 8/30/2011 11:36 PM, Harold Shaw wrote: >>>>>> Can I get a couple of reviews of the changes for: >>>>>> 7082285<http://monaco.us.oracle.com/detail.jsf?cr=7082285> >>>>>> /usr/sbin/installadm-convert dumps traceback with IndexError code >>>>>> >>>>>> The webrev is located at: >>>>>> >>>>>> https://cr.opensolaris.org/action/browse/caiman/hshaw/7082285/webrev/ >>>>>> >>>>>> To verify the fix the following DHCP configurations were run: >>>>>> - Single IP address >>>>>> - Single IP range >>>>>> - Mixture of single IP addresses and IP ranges >>>>>> >>>>>> Thanks, >>>>>> Harold >>>>>> >>>>>> >>>>>> _______________________________________________ >>>>>> caiman-discuss mailing list >>>>>> [email protected] >>>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>>> >>>> _______________________________________________ >>>> caiman-discuss mailing list >>>> [email protected] >>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>> >>> >>> _______________________________________________ >>> caiman-discuss mailing list >>> [email protected] >>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss > _______________________________________________ caiman-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
