On Sunday 17 October 2010 13:05, Vladislav Grishenko wrote:
> > > > 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 subtle 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.
>
> I see.
> Just have to note that bloatcheck shows +64 now with ip_ofs variable vs +8
> without
> with our mipsel-uclibc-gcc 4.3.5
Yes, it's true. I tend to think it's gcc's fault.
--
vda
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
