Hello, > This looks like a anti-improvement wrt code readability. > Why do you do this change at all? The change isn't needed for your patch. Kept as is, only jump to the address post-increment was added.
> -//TODO: DHCP servers do not always sit on the same subnet as clients: > should *ping*, not arp-ping! > + /* TODO: DHCP servers do not always sit on > the same > + * subnet as clients: should *ping*, not > arp-ping! > + */ > > Please don't so this. The comment meant to stand out, because it indicates a > bug we need to fix. Roger that Best Regards, theMIROn ICQ: 303357 Skype: the.miron > -----Original Message----- > From: Denys Vlasenko [mailto:[email protected]] > Sent: Sunday, January 30, 2011 7:48 PM > To: [email protected] > Cc: Vladislav Grishenko; Leonid Lisovskiy > Subject: Re: dhcpd: mac-based dynamic ip address picking > > On Sunday 30 January 2011 12:55, Vladislav Grishenko wrote: > > Hello, > > > > The idea of this patch taken from dnsmasq: > > * the same ip address offering for clients with expired lease (almost > > always) without any additional state saving > > Every time you take lease_epoch++ branch, all future IPs will shift one > address up, and you lose this property. > > > > * prevent the same ip address allocation for different interfaces of > > the same station. > > > > dumb Windows dhcp-client (win7) ACKs same-subnet IP address, no > matter > > if it was previously assigned to the other interfaces, and can't > > assign it to the current one, even if that different interface is down. > > (1) Isn't it a bug in Win7? > (2) What will happen if hash of both interfaces'MACs results in the same IP > being picked? > > > > mac-based allocation makes this "endless discover-offer-request-ack loop" > > issue almost impossible. > > > - /* ie, 192.168.55.0 */ > - if ((addr & 0xff) == 0) > - continue; > - /* ie, 192.168.55.255 */ > - if ((addr & 0xff) == 0xff) > - continue; > - nip = htonl(addr); > - /* is this a static lease addr? */ > - if (is_nip_reserved(server_config.static_leases, nip)) > - continue; > - > + if ( > + /* ie, not 192.168.55.0 */ > + ((addr & 0xff) != 0) && > + /* ie, not 192.168.55.255 */ > + ((addr & 0xff) != 0xff) && > + /* is this not a static lease addr? */ > + !(is_nip_reserved(server_config.static_leases, nip = > htonl(addr))) > + ) { > > This looks like a anti-improvement wrt code readability. > Why do you do this change at all? The change isn't needed for your patch. > > > > -//TODO: DHCP servers do not always sit on the same subnet as clients: > should *ping*, not arp-ping! > + /* TODO: DHCP servers do not always sit on > the same > + * subnet as clients: should *ping*, not > arp-ping! > + */ > > Please don't so this. The comment meant to stand out, because it indicates a > bug we need to fix. > > -- > vda
0003.patch
Description: Binary data
_______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
