🐞 fix: 修复fd泄露v1.0.8-20230403

2 files changed, 42 insertions, 50 deletions

diff --git a/common/src/bfd.cpp b/common/src/bfd.cpp
index d1112e8..3adb44e 100644
--- a/common/src/bfd.cpp
+++ b/common/src/bfd.cpp
@@ -59,22 +59,25 @@ int bfd_vtysh_connect(struct bfd_vtysh_client *client)
 
     if (client->pre_config) {
         ret = client->pre_config(client);
-        if (ret < 0)
+        if (ret < 0) {
+            LOG_DEBUG("bfd vtysh_connect(%s) preconfig error: connect = %s", client->path, strerror(errno));
             bfd_vtysh_close(client);
+            return -1;
+        }
     }
 
     if (client->recover_config) {
         ret = client->recover_config(client);
-        if (ret < 0)
+        if (ret < 0) {
+            LOG_DEBUG("bfd vtysh_connect(%s) recover config error: connect = %s", client->path, strerror(errno));
             bfd_vtysh_close(client);
+            return -1;
+        }
     }
-
     LOG_DEBUG("bfd vtysh_connect(%d): succ", client->fd);
 	return 0;
 }
 
-// return  0 : success
-// return -1 : fail
 void bfd_vtysh_close(struct bfd_vtysh_client *client)
 {
 	if (client->fd > 0) {
@@ -149,6 +152,9 @@ static int bfd_vtysh_cmd_exec(struct bfd_vtysh_client *client, const char *cmd,
     char *bufvalid;
 	char *buf = data;
 
+    if (client->fd <= 0)
+        return -1;
+
     ret = bfd_vtysh_client_send(client, cmd);
     if (ret < 0)
         goto out_err;
@@ -166,33 +172,9 @@ static int bfd_vtysh_cmd_exec(struct bfd_vtysh_client *client, const char *cmd,
 		}
 
         break;
-
-        // LOG_DEBUG("bfd cmd [%s] nread[%d]", cmd, nread);
-        // bufvalid += nread;
-        // bufvalid[0] = '\0';
-        // if (bufvalid - buf >= 4)
-		// 	end = (char*)memmem(bufvalid - 4, 4, "\0", 1);
-        // size_t textlen = (end ? end : bufvalid) - buf;
-
-        // LOG_DEBUG("bfd cmd [%s] buf[%s]", cmd, buf);
-        // memmove(buf, buf + textlen, bufvalid - buf - textlen);
-        // bufvalid -= textlen;
-        // if (end)
-        //     end -= textlen;
-
-        // assert(((buf == bufvalid)
-		// 		|| (bufvalid - buf <= 4 && buf[0] == 0x00)));
-
-        // if (end && bufvalid - buf == 4) {
-        //     if (memcmp(buf, terminator, 3))
-        //         goto out_err;
-		// 	ret = buf[3];
-		// 	break;
-		// }
     } while (1);
     return 0;
 out_err:
-    bfd_vtysh_close(client);
     return -1;
 }
 
@@ -203,6 +185,9 @@ int bfd_vtysh_pre_config(struct bfd_vtysh_client *client)
     int ret = 0;
     char stackbuf[4096];
 
+    if (client->fd <= 0)
+        return -1;
+
     ret = bfd_vtysh_cmd_exec(client, "enable", stackbuf, sizeof(stackbuf)-1);
     if (ret < 0) {
         LOG_DEBUG("bfd vtysh cmd [enable] error");
@@ -229,6 +214,9 @@ int bfd_vtysh_add_dev(struct bfd_vtysh_client *client, const char *peer_addr, in
     char cmd[256] = {0};
     char stackbuf[4096];
 
+    if (client->fd <= 0)
+        return -1;
+
     snprintf(cmd, sizeof(cmd), "peer %s", peer_addr);
     ret = bfd_vtysh_cmd_exec(client, cmd, stackbuf, sizeof(stackbuf)-1);
     if (ret < 0) {
@@ -268,6 +256,9 @@ int bfd_vtysh_del_dev(struct bfd_vtysh_client *client, const char *peer_addr)
     char cmd[256] = {0};
     char stackbuf[4096];
 
+    if (client->fd <= 0)
+        return -1;
+
     snprintf(cmd, sizeof(cmd), "no peer %s", peer_addr);
     ret = bfd_vtysh_cmd_exec(client, cmd, stackbuf, sizeof(stackbuf)-1);
     if (ret < 0) {
diff --git a/platform/src/health_check.cpp b/platform/src/health_check.cpp
index 2698518..dcb4913 100644
--- a/platform/src/health_check.cpp
+++ b/platform/src/health_check.cpp
@@ -239,7 +239,7 @@ uint64_t health_check_session_add(int profile_id, const struct health_check *pol
         bfd_vtysh_close(&client);
     }
 
-    LOG_DEBUG("health check session table insert: session id [%lu] success", session_id);
+    LOG_DEBUG("health check session table insert: profile id [%d] session id [%lu] address [%s] success", profile_id, session_id, policy->address);
     return session_id;
 }
 
@@ -338,22 +338,18 @@ static int get_mac_by_addr(char *addr, uint8_t *buf)
     snprintf(arp_req.arp_dev, IFNAMSIZ, hc_dev_name);
 
     sfd = socket(AF_INET, SOCK_DGRAM, 0);
-
-    saved_errno = errno;
-    ret = ioctl(sfd, SIOCGARP, &arp_req);
-    if (ret < 0) {
-        LOG_ERROR("Get IP [%s] MAC failed : %s", addr, strerror(errno));
+    if (sfd == -1)
         return -1;
-    }
-    errno = saved_errno;
 
-    if (arp_req.arp_flags & ATF_COM)
-        memcpy(buf, arp_req.arp_ha.sa_data, HC_MAC_LEN);        
-    else
+    ret = ioctl(sfd, SIOCGARP, &arp_req);
+    if (ret < 0)
         memcpy(buf, default_gw_mac, HC_MAC_LEN);
+    else
+        memcpy(buf, arp_req.arp_ha.sa_data, HC_MAC_LEN);
 
-    LOG_DEBUG("IP:%s, MAC: %02x:%02x:%02x:%02x:%02x:%02x\n",
+    LOG_DEBUG("IP:%s, MAC: %02x:%02x:%02x:%02x:%02x:%02x",
                 addr, buf[0], buf[1], buf[2], buf[3], buf[4], buf[5]);
+    close(sfd);
     return 0;
 }
 
@@ -381,21 +377,26 @@ static void *_health_check_session_foreach(void *arg)
             if (node->policy.method != HEALTH_CHECK_METHOD_BFD)
                 continue;
 
-            is_active = bfd_vtysh_get_dev_active(&client, node->policy.address);
-            if (is_active == -1) {
-                bfd_vtysh_close(&client);
-                health_check_session_recover_bfd(&client);
-                bfd_vtysh_connect(&client);
+            if (strlen(node->policy.address) != 0) {
                 is_active = bfd_vtysh_get_dev_active(&client, node->policy.address);
-                if (is_active == -1)
-                    is_active = 0;
+                if (is_active == -1) {
+                    bfd_vtysh_close(&client);
+                    health_check_session_recover_bfd(&client);
+                    bfd_vtysh_connect(&client);
+                    is_active = bfd_vtysh_get_dev_active(&client, node->policy.address);
+                    if (is_active == -1)
+                        is_active = 0;
+                }
             }
-            
+            else {
+                is_active = 0;
+            }
+
             sf_status_update(g_sf_status, node->profile_id, is_active, 0);
             if (node->is_active != is_active) {
                 node->is_active = is_active;
                 if (node->is_active == 1) {
-                    get_mac_by_addr(node->policy.address, node->mac);                    
+                    get_mac_by_addr(node->policy.address, node->mac);
                 }
                 else {
                     memset(node->mac, 0, sizeof(node->mac));