Merge pull request #194 from opensourcerouting/coverity-lib-2

more lib/ coverity fixes

lib/command.c

@@ -1450,7 +1450,8 @@ DEFUN (config_write,
                    VTY_NEWLINE);
           goto finished;
         }
-      fsync (dirfd);
+      if (dirfd >= 0)
+        fsync (dirfd);
     }
   if (rename (config_file_tmp, config_file) != 0)
     {
@@ -1458,7 +1459,8 @@ DEFUN (config_write,
                VTY_NEWLINE);
       goto finished;
     }
-  fsync (dirfd);
+  if (dirfd >= 0)
+    fsync (dirfd);
 
   vty_out (vty, "Configuration saved to %s%s", config_file,
            VTY_NEWLINE);
@@ -1467,7 +1469,8 @@ DEFUN (config_write,
 finished:
   if (ret != CMD_SUCCESS)
     unlink (config_file_tmp);
-  close (dirfd);
+  if (dirfd >= 0)
+    close (dirfd);
   XFREE (MTYPE_TMP, config_file_tmp);
   XFREE (MTYPE_TMP, config_file_sav);
   return ret;

lib/csv.c

@@ -177,8 +177,11 @@ csv_decode_record(csv_record_t *rec)
         field = strpbrk(curr, ",");
     }
     field = strstr(curr, "\n");
+    if (!field)
+        return;
+
     fld = malloc(sizeof(csv_field_t));
-    if (field && fld) {
+    if (fld) {
         fld->field = curr;
         fld->field_len = field-curr;
         TAILQ_INSERT_TAIL(&(rec->fields), fld, next_field);
@@ -420,6 +423,7 @@ csv_clone_record (csv_t *csv, csv_record_t *in_rec, csv_record_t **out_rec)
     curr = calloc(1, csv->buflen);
     if (!curr) {
         log_error("field str malloc failed\n");
+        free(rec);
         return;
     }
     rec->record = curr;

lib/grammar_sandbox.c

@@ -86,6 +86,11 @@ DEFUN (grammar_test_complete,
     return CMD_SUCCESS;
 
   vector command = cmd_make_strvec (cmdstr);
+  if (!command)
+    {
+      XFREE (MTYPE_TMP, cmdstr);
+      return CMD_SUCCESS;
+    }
 
   // generate completions of user input
   struct list *completions;
@@ -121,7 +126,7 @@ DEFUN (grammar_test_complete,
   // free resources
   list_delete (completions);
   cmd_free_strvec (command);
-  free (cmdstr);
+  XFREE (MTYPE_TMP, cmdstr);
 
   return CMD_SUCCESS;
 }
@@ -138,7 +143,14 @@ DEFUN (grammar_test_match,
     return CMD_SUCCESS;
 
   char *cmdstr = argv_concat(argv, argc, idx_command);
+  if (!cmdstr)
+    return CMD_SUCCESS;
   vector command = cmd_make_strvec (cmdstr);
+  if (!command)
+    {
+       XFREE (MTYPE_TMP, cmdstr);
+       return CMD_SUCCESS;
+    }
 
   struct list *argvv = NULL;
   const struct cmd_element *element = NULL;
@@ -177,7 +189,7 @@ DEFUN (grammar_test_match,
 
   // free resources
   cmd_free_strvec (command);
-  free (cmdstr);
+  XFREE (MTYPE_TMP, cmdstr);
 
   return CMD_SUCCESS;
 }

lib/mpls.h

@@ -123,59 +123,40 @@ mpls_lse_decode (mpls_lse_t lse, mpls_label_t *label,
 
 /* Printable string for labels (with consideration for reserved values). */
 static inline char *
-label2str (mpls_label_t label, char *buf, int len)
+label2str (mpls_label_t label, char *buf, size_t len)
 {
   switch(label) {
   case MPLS_V4_EXP_NULL_LABEL:
-    strncpy(buf, "IPv4 Explicit Null", len);
+    strlcpy(buf, "IPv4 Explicit Null", len);
     return(buf);
-    break;
   case MPLS_RA_LABEL:
-    strncpy(buf, "Router Alert", len);
+    strlcpy(buf, "Router Alert", len);
     return(buf);
-    break;
   case MPLS_V6_EXP_NULL_LABEL:
-    strncpy(buf, "IPv6 Explict Null", len);
+    strlcpy(buf, "IPv6 Explict Null", len);
     return(buf);
-    break;
   case MPLS_IMP_NULL_LABEL:
-    strncpy(buf, "implicit-null", len);
+    strlcpy(buf, "implicit-null", len);
     return(buf);
-    break;
   case MPLS_ENTROPY_LABEL_INDICATOR:
-    strncpy(buf, "Entropy Label Indicator", len);
+    strlcpy(buf, "Entropy Label Indicator", len);
     return(buf);
-    break;
   case MPLS_GAL_LABEL:
-    strncpy(buf, "Generic Associated Channel", len);
+    strlcpy(buf, "Generic Associated Channel", len);
     return(buf);
-    break;
   case MPLS_OAM_ALERT_LABEL:
-    strncpy(buf, "OAM Alert", len);
+    strlcpy(buf, "OAM Alert", len);
     return(buf);
-    break;
   case MPLS_EXTENSION_LABEL:
-    strncpy(buf, "Extension", len);
+    strlcpy(buf, "Extension", len);
     return(buf);
-    break;
-  case 4:
-  case 5:
-  case 6:
-  case 8:
-  case 9:
-  case 10:
-  case 11:
-  case 12:
-    strncpy(buf, "Reserved", len);
-    return(buf);
-    break;
   default:
-    sprintf(buf, "%u", label);
+    if (label < 16)
+      snprintf(buf, len, "Reserved (%u)", label);
+    else
+      snprintf(buf, len, "%u", label);
     return(buf);
   }
-
-  strncpy(buf, "Error", len);
-  return(buf);
 }
 
 /* constants used by ldpd */

lib/skiplist.c

@@ -439,7 +439,8 @@ skiplist_next_value(
 	return -1;
 
     *valuePointer = q->value;
-    *cursor = q;
+    if (cursor)
+	*cursor = q;
     CHECKLAST(l);
     return 0;
 }

lib/vty.c

@@ -1993,7 +1993,7 @@ vty_serv_un (const char *path)
   /* Make server socket. */
   memset (&serv, 0, sizeof (struct sockaddr_un));
   serv.sun_family = AF_UNIX;
-  strncpy (serv.sun_path, path, strlen (path));
+  strlcpy (serv.sun_path, path, sizeof (serv.sun_path));
 #ifdef HAVE_STRUCT_SOCKADDR_UN_SUN_LEN
   len = serv.sun_len = SUN_LEN(&serv);
 #else
@@ -2357,7 +2357,6 @@ vty_use_backup_config (char *fullpath)
 {
   char *fullpath_sav, *fullpath_tmp;
   FILE *ret = NULL;
-  struct stat buf;
   int tmp, sav;
   int c;
   char buffer[512];
@@ -2365,7 +2364,9 @@ vty_use_backup_config (char *fullpath)
   fullpath_sav = malloc (strlen (fullpath) + strlen (CONF_BACKUP_EXT) + 1);
   strcpy (fullpath_sav, fullpath);
   strcat (fullpath_sav, CONF_BACKUP_EXT);
-  if (stat (fullpath_sav, &buf) == -1)
+
+  sav = open (fullpath_sav, O_RDONLY);
+  if (sav < 0)
     {
       free (fullpath_sav);
       return NULL;
@@ -2377,47 +2378,32 @@ vty_use_backup_config (char *fullpath)
   /* Open file to configuration write. */
   tmp = mkstemp (fullpath_tmp);
   if (tmp < 0)
-    {
-      free (fullpath_sav);
-      free (fullpath_tmp);
-      return NULL;
-    }
+    goto out_close_sav;
 
-  sav = open (fullpath_sav, O_RDONLY);
-  if (sav < 0)
-    {
-      unlink (fullpath_tmp);
-      free (fullpath_sav);
-      free (fullpath_tmp);
-      return NULL;
-    }
+  if (fchmod (tmp, CONFIGFILE_MASK) != 0)
+    goto out_close;
 
   while((c = read (sav, buffer, 512)) > 0)
     {
       if (write (tmp, buffer, c) <= 0)
-        {
-          free (fullpath_sav);
-          free (fullpath_tmp);
-          close (sav);
-          close (tmp);
-          return NULL;
-        }
+        goto out_close;
     }
   close (sav);
   close (tmp);
 
-  if (chmod(fullpath_tmp, CONFIGFILE_MASK) != 0)
-    {
-      unlink (fullpath_tmp);
-      free (fullpath_sav);
-      free (fullpath_tmp);
-      return NULL;
-    }
-
-  if (link (fullpath_tmp, fullpath) == 0)
+  if (rename (fullpath_tmp, fullpath) == 0)
     ret = fopen (fullpath, "r");
+  else
+    unlink (fullpath_tmp);
 
-  unlink (fullpath_tmp);
+  if (0)
+    {
+out_close:
+      close (tmp);
+      unlink (fullpath_tmp);
+out_close_sav:
+      close (sav);
+    }
 
   free (fullpath_sav);
   free (fullpath_tmp);

lib/zclient.c

@@ -35,6 +35,7 @@
 #include "nexthop.h"
 
 DEFINE_MTYPE_STATIC(LIB, ZCLIENT, "Zclient")
+DEFINE_MTYPE_STATIC(LIB, REDIST_INST, "Redistribution instance IDs")
 
 /* Zebra client events. */
 enum event {ZCLIENT_SCHEDULE, ZCLIENT_READ, ZCLIENT_CONNECT};
@@ -106,7 +107,7 @@ redist_add_instance (struct redist_proto *red, u_short instance)
   if (!red->instances)
     red->instances = list_new();
 
-  in = calloc (1, sizeof(u_short));
+  in = XMALLOC (MTYPE_REDIST_INST, sizeof(u_short));
   *in = instance;
   listnode_add (red->instances, in);
 }
@@ -121,7 +122,7 @@ redist_del_instance (struct redist_proto *red, u_short instance)
     return;
 
   listnode_delete(red->instances, id);
-  free (id);
+  XFREE (MTYPE_REDIST_INST, id);
   if (!red->instances->count)
     {
       red->enabled = 0;