Hi, Patrik > -----Original Message----- > From: Patrik Flykt [mailto:[email protected]] > Sent: Thursday, January 23, 2014 8:51 PM > To: Zhang, Zhengguang > Cc: Guoqiang Liu; [email protected]; [email protected]; > [email protected]; [email protected] > Subject: Re: [PATCH] Add REBOOTING state support for dhcp client > > > Hi, > > On Wed, 2014-01-22 at 15:31 +0000, Zhang, Zhengguang wrote: > > I don't suggest that the code should be factored in with > > send_select(), the reason is: > > send_select() is called when dhcp client has received DHCP_OFFER > > message from dhcp server, so dhcp client broadcasts a message with the > > server identifier to notify that it has selected a dhcp server. > > send_rebooting() does a totally different thing: in INIT_REBOOT state, > > dhcp client broadcasts a message with the requested IP address to > > request the previous allocated IP address directly, the max DHCP size > > should be set and "server identifier" must not be filled in. From > > logic, send_rebooting() has nothing to do with send_select(), although > > the codes are somewhere similar, it should not be combined into one > > function, otherwise, it will make the codes very confusing and not > > readable . > > The only difference sending a Request is the server id when not in init-reboot > state. Surely that difference can be handled by checking dhcp_client->state > and > reusing the rest of the identical code? IMHO the function should also be > called > send_request(), as that's what it really does. > 1. Through the code analysis, currently, send_select() is only called when dhcp client has received the DHCPOFFER message, so it's obvious that it is used to select a dhcp server. 2. Take the function send_rebound() as a reference, it's also to broadcast a request message, and the codes are also similar with send_select(), but it is implemented in a separated function. send_rebooting() has 2 lines difference with send_select(): server id and max DHCP size set. send_rebound() has only about 3 lines difference with send_select(). 3. If we change the function send_select() to send_request(), it is OK to combine send_rebooting() and send_rebound() into this function, and some codes won't be implemented repeatedly, but it loses some code readability.
So I prefer to implement them in different functions instead of one function: send_request(), so that we can understand what it intends to do clearly just from the function name. Which solution would you prefer? > Cheers, > > Patrik > Regards Zhang Zhengguang _______________________________________________ connman mailing list [email protected] https://lists.connman.net/mailman/listinfo/connman
