First of all, thank you for code review. I'll try to follow all your recommendations and will prepare next version. Below is few open questions:
>On Tue, Oct 18, 2011 at 3:06 AM, Denys Vlasenko <[email protected]> >wrote: >dhcp6c_script.c: > /* since we count total length above, it is > safely to use strcat() */ > switch (v->lvtype) { > case DHCP6_LISTVAL_VBUF: > strcat(*curr, v->val_vbuf.dv_buf); > strcat(*curr, " "); >strcat is inefficient. strcat(" ") doubly so: >can use *p++ = ' ' instead. Are you sure that following code will be much efficient: len = strlen(*curr); *(*curr + len++) = ' '; *(*curr + len++) = '\0'; Or I miss something? Moreover, I saw similar code in editors/vi.c: strcat(readbuffer, " "); >On Tue, Oct 18, 2011 at 5:04 PM, Denys Vlasenko <[email protected]> >wrote: >> Are use of getifaddrs/freeifaddrs prohibited in bb? >I guess it's not a very good idea to use them, they are not portable. >Why dhch6 even needs to know some intimate things about interfaces? >I only understand the need to know MAC, nothing else. There are two places of getifaddrs() dhcp6c use: 1) if6.c:if6init() - it tries to determine existent IPv6 address on listen interface. Used in server only, so must be moved to server code. 2) if6.c:gethwid() - determine MAC, will be replaced to SIOCGIFCONF + udhcp_read_interface(). Iteration through interfaces needed since ppp0, for example, hasn't MAC. >BTW, udhcpc is handling just one iface, which simplifies logic a lot. >What do you think about using the same approach for DHCPv6? I apologize that it is definitely acceptable for DHCPv6 client, but I'm not sure for server. >+ /* find the corresponding event based on the received xid */ >+ ev = find_event_withid(ifp->ifid, ntohl(dh6->dh6_xid) & DH6_XIDMASK); >+ if (ev == NULL) { >+ bb_error_msg("XID mismatch"); >+ goto fail; >+ } >Why we emit a message? It's just a packet to somebody else. >I think it needs to be log1()... We receive a valid DHCPv6 message on our interface, correct multicast address and message type, but XID isn't our. This can be occur either if we have another DHCPv6 client/relay/server on same host/interface, or it is a bug. Not enough reason for error message? _______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
