> From: Denys Vlasenko > Sent: Sunday, October 17, 2010 4:27 PM > Cc: Vladislav Grishenko > Subject: Re: [PATCH] udhcpc: fixed option_ip_pair parsing > > On Sunday 17 October 2010 10:25, Vladislav Grishenko wrote: > > Hello, > > Sorry, previous message doesn't include description. > > > > Commit > > > http://git.busybox.net/busybox/commit/?id=7d3a48a003cd645edfae2b4044 > 93 > > 688022 > > b13193 > > revealed incorrect OPTION_IP_PAIR implementation, which doesn't > > respect option length and causes erroneous classful routes, composed > > from garbage or first bytes from the next DHCP packet option. > > Patch is attached. > > Good catch! > I propose the fix which is less mind-bending: remove all +/-4 tricks in > OPTION_IP_PAIR (they are hard to follow and that's why they contained a > sublte bug), and add ip_ofs variable which may be used to advance "option" > pointer in OPTION_IP: > > while (len >= optlen) { > + unsigned ip_ofs = 0; > > switch (type) { > case OPTION_IP_PAIR: > dest += sprint_nip(dest, "", option); > *dest++ = '/'; > - option += 4; > - optlen = 4; > + ip_ofs = 4; > /* fall through */ > case OPTION_IP: > - dest += sprint_nip(dest, "", option); > + dest += sprint_nip(dest, "", option + ip_ofs); > break; > > > -- > vda
Yep, it looks cute. Denys, do we really need to zero ip_opt in each loop? Option type remains const in this function. Best Regards, theMIROn ICQ: 303357 Skype: the.miron _______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
