On Sunday 17 October 2010 12:37, Vladislav Grishenko wrote:
> > > 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;
> >
> >
>
> Yep, it looks cute.
> Denys, do we really need to zero ip_opt in each loop? Option type remains
> const in this function.
As it coded now, clever compiler can figure out that ip_ofs is never used
past "case OPTION_IP", and can reuse its CPU register for something else.
With ip_ofs moved out-of-loop, compiler will not be able to understand this,
and will retain it for the entire loop body.
--
vda
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
