xiaoxiang781216 commented on code in PR #3131:
URL: https://github.com/apache/nuttx-apps/pull/3131#discussion_r2204058431
##########
netutils/dhcpc/dhcpc.c:
##########
@@ -87,6 +87,14 @@
#define DHCPNAK 6
#define DHCPRELEASE 7
+#ifndef CONFIG_NETUTILS_DHCPC_RELEASE_RETRIES
+#define CONFIG_NETUTILS_DHCPC_RELEASE_RETRIES 3
+#endif
+
+#ifndef CONFIG_NETUTILS_DHCPC_RELEASE_TRANSMISSION_DELAY_MS
+#define CONFIG_NETUTILS_DHCPC_RELEASE_TRANSMISSION_DELAY_MS 10
+#endif
Review Comment:
remove all fallback, let's trust build environment.
##########
netutils/dhcpc/dhcpc.c:
##########
@@ -950,3 +968,107 @@ int dhcpc_request_async(FAR void *handle,
dhcpc_callback_t callback)
return OK;
}
+
+/****************************************************************************
+ * Name: dhcpc_release
+ ****************************************************************************/
+
+int dhcpc_release(FAR void *handle, FAR struct dhcpc_state *presult)
+{
+ FAR struct dhcpc_state_s *pdhcpc = (FAR struct dhcpc_state_s *)handle;
+ int ret;
+ int send_attempts = 0;
Review Comment:
```suggestion
int retries = 0;
int ret
```
##########
netutils/dhcpc/dhcpc.c:
##########
@@ -950,3 +968,107 @@ int dhcpc_request_async(FAR void *handle,
dhcpc_callback_t callback)
return OK;
}
+
+/****************************************************************************
+ * Name: dhcpc_release
+ ****************************************************************************/
+
+int dhcpc_release(FAR void *handle, FAR struct dhcpc_state *presult)
+{
+ FAR struct dhcpc_state_s *pdhcpc = (FAR struct dhcpc_state_s *)handle;
+ int ret;
+ int send_attempts = 0;
+ const int max_attempts = CONFIG_NETUTILS_DHCPC_RELEASE_RETRIES;
Review Comment:
why not use CONFIG_NETUTILS_DHCPC_RELEASE_RETRIES directly
##########
netutils/dhcpc/dhcpc.c:
##########
@@ -950,3 +968,107 @@ int dhcpc_request_async(FAR void *handle,
dhcpc_callback_t callback)
return OK;
}
+
+/****************************************************************************
+ * Name: dhcpc_release
+ ****************************************************************************/
+
+int dhcpc_release(FAR void *handle, FAR struct dhcpc_state *presult)
+{
+ FAR struct dhcpc_state_s *pdhcpc = (FAR struct dhcpc_state_s *)handle;
+ int ret;
+ int send_attempts = 0;
+ const int max_attempts = CONFIG_NETUTILS_DHCPC_RELEASE_RETRIES;
+
+ if (!handle || !presult)
+ {
+ errno = EINVAL;
+ return ERROR;
+ }
+
+ /* Check that we have valid IP address and server ID to release */
+
+ if (presult->ipaddr.s_addr == 0 || presult->serverid.s_addr == 0)
+ {
+ errno = EINVAL;
+ return ERROR;
+ }
+
+ /* Increment transaction ID for the release message */
+
+ pdhcpc->xid[3]++;
+
+ /* Send DHCPRELEASE message to the server with retry mechanism.
+ * According to RFC 2131, no response is expected from the server.
+ */
+
+ while (send_attempts < max_attempts)
+ {
+ ret = dhcpc_sendmsg(pdhcpc, presult, DHCPRELEASE);
+ if (ret > 0)
+ {
+ ninfo("DHCPRELEASE message sent successfully (%d bytes)\n", ret);
+ break;
+ }
+ else
+ {
+ send_attempts++;
+ nerr("Failed send DHCPRELEASE (attempt %d/%d), ret=%d, errno=%d\n",
+ send_attempts, max_attempts, ret, errno);
+
+ if (send_attempts < max_attempts)
+ {
+ usleep(1000
+ * CONFIG_NETUTILS_DHCPC_RELEASE_TRANSMISSION_DELAY_MS);
+ }
+ }
+ }
+
+ if (send_attempts >= max_attempts)
+ {
+ nerr("ERROR: Failed to send DHCPRELEASE after %d attempts\n",
+ max_attempts);
+ return ERROR;
+ }
+
+#ifdef CONFIG_NETUTILS_DHCPC_RELEASE_ENSURE_TRANSMISSION
+ /* Ensure the DHCPRELEASE packet has time to be transmitted.
+ * Since DHCP RELEASE has no ACK response and UDP is connectionless,
+ * we use a delay to give the network stack time to actually send
+ * the packet before the function returns.
+ */
+
+ usleep(1000 * CONFIG_NETUTILS_DHCPC_RELEASE_TRANSMISSION_DELAY_MS);
+#endif
+
+#ifdef CONFIG_NETUTILS_DHCPC_RELEASE_CLEAR_IP
+ /* Clear all network configuration that was obtained via DHCP */
+
+ struct in_addr zero_addr;
Review Comment:
coding style doesn't allow to define the variable in the middle of block,
let's create a new api or move to the begin of dhcpc_release
##########
netutils/dhcpc/dhcpc.c:
##########
@@ -950,3 +968,107 @@ int dhcpc_request_async(FAR void *handle,
dhcpc_callback_t callback)
return OK;
}
+
+/****************************************************************************
+ * Name: dhcpc_release
+ ****************************************************************************/
+
+int dhcpc_release(FAR void *handle, FAR struct dhcpc_state *presult)
+{
+ FAR struct dhcpc_state_s *pdhcpc = (FAR struct dhcpc_state_s *)handle;
+ int ret;
+ int send_attempts = 0;
+ const int max_attempts = CONFIG_NETUTILS_DHCPC_RELEASE_RETRIES;
+
+ if (!handle || !presult)
+ {
+ errno = EINVAL;
+ return ERROR;
+ }
+
+ /* Check that we have valid IP address and server ID to release */
+
+ if (presult->ipaddr.s_addr == 0 || presult->serverid.s_addr == 0)
+ {
+ errno = EINVAL;
+ return ERROR;
+ }
+
+ /* Increment transaction ID for the release message */
+
+ pdhcpc->xid[3]++;
+
+ /* Send DHCPRELEASE message to the server with retry mechanism.
+ * According to RFC 2131, no response is expected from the server.
+ */
+
+ while (send_attempts < max_attempts)
+ {
+ ret = dhcpc_sendmsg(pdhcpc, presult, DHCPRELEASE);
+ if (ret > 0)
+ {
+ ninfo("DHCPRELEASE message sent successfully (%d bytes)\n", ret);
+ break;
+ }
+ else
+ {
+ send_attempts++;
+ nerr("Failed send DHCPRELEASE (attempt %d/%d), ret=%d, errno=%d\n",
+ send_attempts, max_attempts, ret, errno);
+
+ if (send_attempts < max_attempts)
+ {
+ usleep(1000
+ * CONFIG_NETUTILS_DHCPC_RELEASE_TRANSMISSION_DELAY_MS);
+ }
+ }
+ }
+
+ if (send_attempts >= max_attempts)
Review Comment:
remove the check here and reuse the check at line 1019 to return
##########
netutils/dhcpc/dhcpc.c:
##########
@@ -950,3 +968,107 @@ int dhcpc_request_async(FAR void *handle,
dhcpc_callback_t callback)
return OK;
}
+
+/****************************************************************************
+ * Name: dhcpc_release
+ ****************************************************************************/
+
+int dhcpc_release(FAR void *handle, FAR struct dhcpc_state *presult)
+{
+ FAR struct dhcpc_state_s *pdhcpc = (FAR struct dhcpc_state_s *)handle;
+ int ret;
+ int send_attempts = 0;
+ const int max_attempts = CONFIG_NETUTILS_DHCPC_RELEASE_RETRIES;
+
+ if (!handle || !presult)
+ {
+ errno = EINVAL;
+ return ERROR;
+ }
+
+ /* Check that we have valid IP address and server ID to release */
+
+ if (presult->ipaddr.s_addr == 0 || presult->serverid.s_addr == 0)
+ {
+ errno = EINVAL;
+ return ERROR;
+ }
+
+ /* Increment transaction ID for the release message */
+
+ pdhcpc->xid[3]++;
+
+ /* Send DHCPRELEASE message to the server with retry mechanism.
+ * According to RFC 2131, no response is expected from the server.
+ */
+
+ while (send_attempts < max_attempts)
Review Comment:
let's change `for (; ; )` and reuse the check at line 1019 to exit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
