* fixed bug: we got garbled output on the line with "shape=box"
authorBernd Petrovitsch <bernd@firmix.at>
Fri, 16 Nov 2007 19:52:09 +0000 (19:52 +0000)
committerBernd Petrovitsch <bernd@firmix.at>
Fri, 16 Nov 2007 19:52:09 +0000 (19:52 +0000)
* fixed file descriptor leaks ipc_action()
* killed the two indicator variables "ipc_open" and "ipc_socket_up". We
  use the filedescriptors as we can easily use -1 as the "socket is down"
  indocation
* killed unused return values
* reduced the clutter: we have now a ipc_send_fmt() function which
  does the snprintf() and not spread that all over

lib/dot_draw/src/olsrd_dot_draw.c

index 39a24f3..eccf20e 100644 (file)
@@ -37,7 +37,7 @@
  * to the project. For more information see the website or contact
  * the copyright holders.
  *
- * $Id: olsrd_dot_draw.c,v 1.32 2007/11/16 19:12:55 bernd67 Exp $
+ * $Id: olsrd_dot_draw.c,v 1.33 2007/11/16 19:52:09 bernd67 Exp $
  */
 
 /*
@@ -57,6 +57,7 @@
 #include <stdlib.h>
 #include <unistd.h>
 #include <errno.h>
+#include <stdarg.h>
 
 #include "olsr.h"
 #include "olsr_types.h"
@@ -79,9 +80,7 @@
 
 
 static int ipc_socket;
-static int ipc_open;
 static int ipc_connection;
-static int ipc_socket_up;
 
 
 
@@ -108,11 +107,13 @@ ipc_print_tc_link(const struct tc_entry *entry, const struct tc_edge_entry *dst_
 static void
 ipc_print_net(const union olsr_ip_addr *, const union olsr_ip_addr *, const union hna_netmask *);
 
-static int
+static void
 ipc_send(const char *, int);
 
-static int
-ipc_send_str(const char *);
+static void
+ipc_send_fmt(const char *format, ...) __attribute__((format(printf,1,2)));
+
+#define ipc_send_str(data) ipc_send((data), strlen(data))
 
 
 /**
@@ -125,8 +126,6 @@ int
 olsrd_plugin_init(void)
 {
   /* Initial IPC value */
-  ipc_open = 0;
-  ipc_socket_up = 0;
   ipc_socket = -1;
   ipc_connection = -1;
 
@@ -145,39 +144,42 @@ olsrd_plugin_init(void)
 void
 olsr_plugin_exit(void)
 {
-  if(ipc_open)
-    close(ipc_socket);
+  if (ipc_connection != -1) {
+    CLOSE(ipc_connection);
+  }
+  if (ipc_socket != -1) {
+    CLOSE(ipc_socket);
+  }
 }
 
 
 static void
 ipc_print_neigh_link(const struct neighbor_entry *neighbor)
 {
-  char buf[256];
-  struct ipaddr_str strbuf;
+  struct ipaddr_str mainaddrstrbuf, strbuf;
   double etx = 0.0;
-  const char* style = "solid";
+  const char *style;
+  const char *adr = olsr_ip_to_string(&mainaddrstrbuf, &olsr_cnf->main_addr);
   struct link_entry* link;
 
-  sprintf( buf, "\"%s\" -> ", olsr_ip_to_string(&strbuf, &olsr_cnf->main_addr));
-  ipc_send_str(buf);
-  
   if (neighbor->status == 0) { // non SYM
-       style = "dashed";
-  }
-  else {   
-      link = get_best_link_to_neighbor(&neighbor->neighbor_main_addr);
-      if (link) {
-        etx = olsr_calc_link_etx(link);
-      }
+    style = "dashed";
+  } else {   
+    link = get_best_link_to_neighbor(&neighbor->neighbor_main_addr);
+    if (link) {
+      etx = olsr_calc_link_etx(link);
+    }
+    style = "solid";
   }
     
-  sprintf( buf, "\"%s\"[label=\"%.2f\", style=%s];\n", olsr_ip_to_string(&strbuf, &neighbor->neighbor_main_addr), etx, style );
-  ipc_send_str(buf);
+  ipc_send_fmt("\"%s\" -> \"%s\"[label=\"%.2f\", style=%s];\n",
+               adr,
+               olsr_ip_to_string(&strbuf, &neighbor->neighbor_main_addr),
+               etx,
+               style);
   
   if (neighbor->is_mpr) {
-       sprintf( buf, "\"%s\"[shape=box];\n", buf);
-       ipc_send_str(buf);
+    ipc_send_fmt("\"%s\"[shape=box];\n", adr);
   }
 }
 
@@ -188,55 +190,56 @@ plugin_ipc_init(void)
   struct sockaddr_in sin;
   olsr_u32_t yes = 1;
 
+  if (ipc_socket != -1) {
+    close(ipc_socket);
+  }
+
   /* Init ipc socket */
-  if ((ipc_socket = socket(AF_INET, SOCK_STREAM, 0)) == -1) 
-    {
-      olsr_printf(1, "(DOT DRAW)IPC socket %s\n", strerror(errno));
-      return 0;
-    }
-  else
-    {
-      if (setsockopt(ipc_socket, SOL_SOCKET, SO_REUSEADDR, (char *)&yes, sizeof(yes)) < 0) 
-      {
-       perror("SO_REUSEADDR failed");
-       return 0;
-      }
+  ipc_socket = socket(AF_INET, SOCK_STREAM, 0);
+  if (ipc_socket == -1) {
+    olsr_printf(1, "(DOT DRAW)IPC socket %s\n", strerror(errno));
+    return 0;
+  }
+
+  if (setsockopt(ipc_socket, SOL_SOCKET, SO_REUSEADDR, (char *)&yes, sizeof(yes)) < 0) {
+    perror("SO_REUSEADDR failed");
+    CLOSE(ipc_socket);
+    return 0;
+  }
 
 #if defined __FreeBSD__ && defined SO_NOSIGPIPE
-      if (setsockopt(ipc_socket, SOL_SOCKET, SO_NOSIGPIPE, (char *)&yes, sizeof(yes)) < 0) 
-      {
-       perror("SO_REUSEADDR failed");
-       return 0;
-      }
+  if (setsockopt(ipc_socket, SOL_SOCKET, SO_NOSIGPIPE, (char *)&yes, sizeof(yes)) < 0) {
+    perror("SO_REUSEADDR failed");
+    CLOSE(ipc_socket);
+    return 0;
+  }
 #endif
 
-      /* Bind the socket */
+  /* Bind the socket */
       
-      /* complete the socket structure */
-      memset(&sin, 0, sizeof(sin));
-      sin.sin_family = AF_INET;
-      sin.sin_addr.s_addr = INADDR_ANY;
-      sin.sin_port = htons(ipc_port);
+  /* complete the socket structure */
+  memset(&sin, 0, sizeof(sin));
+  sin.sin_family = AF_INET;
+  sin.sin_addr.s_addr = INADDR_ANY;
+  sin.sin_port = htons(ipc_port);
       
-      /* bind the socket to the port number */
-      if (bind(ipc_socket, (struct sockaddr *) &sin, sizeof(sin)) == -1) 
-       {
-         olsr_printf(1, "(DOT DRAW)IPC bind %s\n", strerror(errno));
-         return 0;
-       }
+  /* bind the socket to the port number */
+  if (bind(ipc_socket, (struct sockaddr *) &sin, sizeof(sin)) == -1) {
+    olsr_printf(1, "(DOT DRAW)IPC bind %s\n", strerror(errno));
+    CLOSE(ipc_socket);
+    return 0;
+  }
       
-      /* show that we are willing to listen */
-      if (listen(ipc_socket, 1) == -1) 
-       {
-         olsr_printf(1, "(DOT DRAW)IPC listen %s\n", strerror(errno));
-         return 0;
-       }
-
-      /* Register with olsrd */
-      //printf("Adding socket with olsrd\n");
-      add_olsr_socket(ipc_socket, &ipc_action);
-      ipc_socket_up = 1;
-    }
+  /* show that we are willing to listen */
+  if (listen(ipc_socket, 1) == -1) {
+    olsr_printf(1, "(DOT DRAW)IPC listen %s\n", strerror(errno));
+    CLOSE(ipc_socket);
+    return 0;
+  }
+
+  /* Register with olsrd */
+  //printf("Adding socket with olsrd\n");
+  add_olsr_socket(ipc_socket, &ipc_action);
 
   return 1;
 }
@@ -248,37 +251,22 @@ ipc_action(int fd __attribute__((unused)))
   struct sockaddr_in pin;
   socklen_t addrlen = sizeof(struct sockaddr_in);
 
-  if (ipc_open)
-    {
-      int rc;
-      do {
-        rc = close(ipc_connection);
-      } while (rc == -1 && (errno == EINTR || errno == EAGAIN));
-      if (rc == -1) {
-        olsr_printf(1, "(DOT DRAW) Error on closing previously active TCP connection on fd %d: %s\n", ipc_connection, strerror(errno));
-      }
-      ipc_open = 0;
-    }
+  if (ipc_connection != -1) {
+    close(ipc_connection);
+  }
   
-  if ((ipc_connection = accept(ipc_socket, (struct sockaddr *)  &pin, &addrlen)) == -1)
-    {
-      olsr_printf(1, "(DOT DRAW)IPC accept: %s\n", strerror(errno));
-    }
-  else
-    {
-        if(!ip4equal(&pin.sin_addr, &ipc_accept_ip.v4))
-       {
-         olsr_printf(0, "Front end-connection from foreign host (%s) not allowed!\n", inet_ntoa(pin.sin_addr));
-         close(ipc_connection);
-          ipc_connection = -1;
-       }
-      else
-       {
-         ipc_open = 1;
-         olsr_printf(1, "(DOT DRAW)IPC: Connection from %s\n",inet_ntoa(pin.sin_addr));
-         pcf_event(1, 1, 1);
-       }
-    }
+  ipc_connection = accept(ipc_socket, (struct sockaddr *)&pin, &addrlen);
+  if (ipc_connection == -1) {
+    olsr_printf(1, "(DOT DRAW)IPC accept: %s\n", strerror(errno));
+    return;
+  }
+  if (!ip4equal(&pin.sin_addr, &ipc_accept_ip.v4)) {
+    olsr_printf(0, "Front end-connection from foreign host (%s) not allowed!\n", inet_ntoa(pin.sin_addr));
+    CLOSE(ipc_connection);
+    return;
+  }
+  olsr_printf(1, "(DOT DRAW)IPC: Connection from %s\n", inet_ntoa(pin.sin_addr));
+  pcf_event(1, 1, 1);
 }
 
 
@@ -290,85 +278,69 @@ pcf_event(int changes_neighborhood,
          int changes_topology,
          int changes_hna)
 {
-  int res;
-  int index;
-  struct neighbor_entry *neighbor_table_tmp;
-  struct tc_entry *tc;
-  struct tc_edge_entry *tc_edge;
-
-  res = 0;
-
-  if(changes_neighborhood || changes_topology || changes_hna)
-    {
-      /* Print tables to IPC socket */
-
-      ipc_send_str("digraph topology\n{\n");
-
-      /* Neighbors */
-      for(index=0;index<HASHSIZE;index++)
-       {         
-         for(neighbor_table_tmp = neighbortable[index].next;
-             neighbor_table_tmp != &neighbortable[index];
-             neighbor_table_tmp = neighbor_table_tmp->next)
-           {
-             ipc_print_neigh_link( neighbor_table_tmp );
-           }
-       }
-
-      /* Topology */  
-      OLSR_FOR_ALL_TC_ENTRIES(tc) {
-          OLSR_FOR_ALL_TC_EDGE_ENTRIES(tc, tc_edge) {
-              ipc_print_tc_link(tc, tc_edge);
-          } OLSR_FOR_ALL_TC_EDGE_ENTRIES_END(tc, tc_edge);
-      } OLSR_FOR_ALL_TC_ENTRIES_END(tc);
-
-      /* HNA entries */
-      for(index=0;index<HASHSIZE;index++) {
-          struct hna_entry *tmp_hna;
-         /* Check all entrys */
-         for (tmp_hna = hna_set[index].next; tmp_hna != &hna_set[index]; tmp_hna = tmp_hna->next) {
-             /* Check all networks */
-              struct hna_net *tmp_net;
-             for (tmp_net = tmp_hna->networks.next; tmp_net != &tmp_hna->networks; tmp_net = tmp_net->next) {
-                 ipc_print_net(&tmp_hna->A_gateway_addr, 
-                               &tmp_net->A_network_addr, 
-                               &tmp_net->A_netmask);
-              }
-          }
+  int res = 0;
+  if(changes_neighborhood || changes_topology || changes_hna) {
+    struct neighbor_entry *neighbor_table_tmp;
+    struct tc_entry *tc;
+    struct tc_edge_entry *tc_edge;
+    struct local_hna_entry *hna;
+    int idx;
+    
+    /* Print tables to IPC socket */
+    ipc_send_str("digraph topology\n{\n");
+
+    /* Neighbors */
+    for (idx = 0; idx < HASHSIZE; idx++) {       
+      for(neighbor_table_tmp = neighbortable[idx].next;
+          neighbor_table_tmp != &neighbortable[idx];
+          neighbor_table_tmp = neighbor_table_tmp->next){
+        ipc_print_neigh_link( neighbor_table_tmp );
       }
+    }
+
+    /* Topology */  
+    OLSR_FOR_ALL_TC_ENTRIES(tc) {
+      OLSR_FOR_ALL_TC_EDGE_ENTRIES(tc, tc_edge) {
+        ipc_print_tc_link(tc, tc_edge);
+      } OLSR_FOR_ALL_TC_EDGE_ENTRIES_END(tc, tc_edge);
+    } OLSR_FOR_ALL_TC_ENTRIES_END(tc);
+
+    /* HNA entries */
+    for (idx = 0; idx < HASHSIZE; idx++) {
+      struct hna_entry *tmp_hna;
+      /* Check all entrys */
+      for (tmp_hna = hna_set[idx].next; tmp_hna != &hna_set[idx]; tmp_hna = tmp_hna->next) {
+        /* Check all networks */
+        struct hna_net *tmp_net;
+        for (tmp_net = tmp_hna->networks.next; tmp_net != &tmp_hna->networks; tmp_net = tmp_net->next) {
+          ipc_print_net(&tmp_hna->A_gateway_addr, 
+                        &tmp_net->A_network_addr, 
+                        &tmp_net->A_netmask);
+        }
+      }
+    }
 
-      /* Local HNA entries */
+    /* Local HNA entries */
+    for (hna = olsr_cnf->hna_entries; hna != NULL; hna = hna->next) {
+      union hna_netmask hna_msk;
       if (olsr_cnf->ip_version == AF_INET) {
-          struct local_hna_entry *hna;
-          for (hna = olsr_cnf->hna_entries; hna != NULL; hna = hna->next) {
-              union olsr_ip_addr netmask;
-              union hna_netmask hna_msk;
-              //hna_msk.v4 = hna4->netmask.v4;
-              olsr_prefix_to_netmask(&netmask, hna->net.prefix_len);
-              hna_msk.v4 = netmask.v4.s_addr;
-              ipc_print_net(&olsr_cnf->interfaces->interf->ip_addr,
-                            &hna->net.prefix,
-                            &hna_msk);
-              
-          }
+        union olsr_ip_addr netmask;
+        //hna_msk.v4 = hna4->netmask.v4;
+        olsr_prefix_to_netmask(&netmask, hna->net.prefix_len);
+        hna_msk.v4 = netmask.v4.s_addr;
       } else {
-          struct local_hna_entry *hna;
-          for (hna = olsr_cnf->hna_entries; hna != NULL; hna = hna->next) {
-              union hna_netmask hna_msk;
-              hna_msk.v6 = hna->net.prefix_len;
-              ipc_print_net(&olsr_cnf->interfaces->interf->ip_addr,
-                            &hna->net.prefix,
-                            &hna_msk);
-          }
+        hna_msk.v6 = hna->net.prefix_len;
       }
-
-      ipc_send_str("}\n\n");
-
-      res = 1;
+      ipc_print_net(&olsr_cnf->interfaces->interf->ip_addr,
+                    &hna->net.prefix,
+                    &hna_msk);
     }
+    ipc_send_str("}\n\n");
 
+    res = 1;
+  }
 
-  if (!ipc_socket_up) {
+  if (ipc_socket == -1) {
     plugin_ipc_init();
   }
   return res;
@@ -377,64 +349,58 @@ pcf_event(int changes_neighborhood,
 static void
 ipc_print_tc_link(const struct tc_entry *entry, const struct tc_edge_entry *dst_entry)
 {
-  char buf[512];
   struct ipaddr_str strbuf1, strbuf2;
 
-  sprintf( buf, "\"%s\" -> \"%s\"[label=\"%.2f\"];\n",
-           olsr_ip_to_string(&strbuf1, &entry->addr),
-           olsr_ip_to_string(&strbuf2, &dst_entry->T_dest_addr),
-           olsr_calc_tc_etx(dst_entry));
-  ipc_send_str(buf);
+  ipc_send_fmt("\"%s\" -> \"%s\"[label=\"%.2f\"];\n",
+               olsr_ip_to_string(&strbuf1, &entry->addr),
+               olsr_ip_to_string(&strbuf2, &dst_entry->T_dest_addr),
+               olsr_calc_tc_etx(dst_entry));
 }
 
 
 static void
 ipc_print_net(const union olsr_ip_addr *gw, const union olsr_ip_addr *net, const union hna_netmask *mask)
 {
-  char buf[512];
   struct ipaddr_str gwbuf, netbuf;
 
-  sprintf( buf, "\"%s\" -> \"%s/%s\"[label=\"HNA\"];\n",
-           olsr_ip_to_string(&gwbuf, gw),
-           olsr_ip_to_string(&netbuf, net),
-           olsr_netmask_to_string(mask));
-  ipc_send_str(buf);
+  ipc_send_fmt("\"%s\" -> \"%s/%s\"[label=\"HNA\"];\n",
+               olsr_ip_to_string(&gwbuf, gw),
+               olsr_ip_to_string(&netbuf, net),
+               olsr_netmask_to_string(mask));
 
-  sprintf( buf,"\"%s/%s\"[shape=diamond];\n",
-           olsr_ip_to_string(&netbuf, net),
-           olsr_netmask_to_string(mask));
-  ipc_send_str(buf);
+  ipc_send_fmt("\"%s/%s\"[shape=diamond];\n",
+               olsr_ip_to_string(&netbuf, net),
+               olsr_netmask_to_string(mask));
 }
 
-static int
-ipc_send_str(const char *data)
-{
-  if(!ipc_open)
-    return 0;
-  return ipc_send(data, strlen(data));
-}
-
-
-static int
+static void
 ipc_send(const char *data, int size)
 {
-  if(!ipc_open)
-    return 0;
-
+  if (ipc_connection != -1) {
 #if defined __FreeBSD__ || defined __NetBSD__ || defined __OpenBSD__ || defined __MacOSX__
 #define FLAGS 0
 #else
 #define FLAGS MSG_NOSIGNAL
 #endif
-  if (send(ipc_connection, data, size, FLAGS) == -1)
-    {
+    if (send(ipc_connection, data, size, FLAGS) == -1) {
       olsr_printf(1, "(DOT DRAW)IPC connection lost!\n");
-      close(ipc_connection);
-      ipc_open = 0;
-      return -1;
+      CLOSE(ipc_connection);
     }
+  }
+}
 
-  return 1;
+static void
+ipc_send_fmt(const char *format, ...)
+{
+  if (ipc_connection != -1) {
+    char buf[4096];
+    int len;
+    va_list arg;
+    va_start(arg, format);
+    len = vsnprintf(buf, sizeof(buf), format, arg);
+    va_end(arg);
+    ipc_send(buf, len);
+  }
 }
 
 static char*
@@ -443,7 +409,7 @@ olsr_netmask_to_string(const union hna_netmask *mask)
   char *ret;
   struct in_addr in;
 
-  if(olsr_cnf->ip_version == AF_INET) {
+  if (olsr_cnf->ip_version == AF_INET) {
       in.s_addr = mask->v4;
       ret = inet_ntoa(in);
   } else {