[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: DHCPv4-o-DHCPv6 with udhcp client
From:       Leonardo Jelenkovic <leonardo.jelenkovic () fer ! hr>
Date:       2014-06-22 15:32:14
Message-ID: CANJyNKtjUU5LOBxU_-oByzZ9FBZoRKtEvDRtRv48wM-Pxza2Nw () mail ! gmail ! com
[Download RAW message or body]

On 16 June 2014 02:15, Denys Vlasenko <vda.linux@googlemail.com> wrote:
[cut]
>
> I propose renaming FEATURE_DHCP4o6C to FEATURE_DHCP4_OVER6
> as a bit more readable.

Maybe FEATURE_DHCP4_OVER_DHCP6 ? A bit long but descriptive.

>
> You are adding another "static void *d6_find_option(...)" in dhcp4o6.c.
> Having two static functions with the same name and similar functionality
> in two different files is not a good practice.
> Can you improve existing function so that it does what you need?
> If not, add a comment why it is impossible or very difficult.
> Same goes for a few other d6_foo functions you reimplement.
>

I use only four functions from d6_dhcpc.c, that is I copied them and modified.

Function original d6_find_option assumes options length is less than
255 (although length field is 16 bit long). Our version uses all
16-bits (it could replace one in d6_dhcpc.c).

Other functions can be adapted (fixed and with "#if"s) and used
directly from d6_dhcpc.c.
However, since we use only four functions from d6_dhcpc.c is it
necessary to include whole file? We can use #if-s but ...

Another similar question: we put some additional code to separate
files (dhcp4o6.*). However, since added code is small in size I think
it would be best to merge this with dhcpc.* (append at end, placed
between #if/#endif). What is your opinion/policy on that?

>
> "#if 1 /* not working! */" part looks confusing. If it doesn't work,
> why is it enabled?
>

We fixed code, but forgot to remove comment.

I agree with rest of your comments.

[cut]
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic