Just to close the loop, the decision has been made to put back as is. I appreciate everyone's feedback and review comments.

Thanks,
Harold

On 09/02/11 13:48, Jesse Butler wrote:
On Sep 2, 2011, at 2:04 PM, Dave Miner wrote:

On 09/02/11 13:34, Harold Shaw wrote:
Dave,
The code here is attempting to create the ISC DHCP subnet stanza from
the existing Solaris DHCP configuration. It may be possible to get a
range of addresses from 'pntadn -P<network>' of
192.168.12.240-192.168.13.32. Instead of generating a single range I
chose to generate it as 2 ranges 192.168.12.240 - 192.168.12.254 and
192.168.13.0 - 192.168.13.32. It was easier to do and functionally
equivalent. I can make the change necessary to generate this as a single
range if desired.

So the generated configuration has multiple ranges rather than a single one?  
That seems sub-optimal to me.  I at least understand why the code is correct, 
anyway.
FWIW, I would tend to agree here that we should have one entry for the range. 
While functionally equivalent, it just seems a bit confusing (and 
wrong-seeming) to work with the tuples as meaningful network/client delineation 
bounds. That said, I also see that it will indeed be functionally equivalent, 
so if this is the best way to do it for now, I'm not entirely opposed to it. 
But, like I said yesterday, it may lead to confusion. The comment certainly 
adds clarity, though, so I'm Ok with it.
/jb

Dave

Harold

On 09/02/11 10:26, Dave Miner wrote:
Harold, I don't follow how "treating addresses as Class C" is at all
correct here. Can you elaborate on why that's the right thing to do
when networks of that particular size are increasingly uncommon?

Dave

On 09/01/11 17: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
_______________________________________________
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

Reply via email to