Improve rhizome manifest debugging

Move rhizome_new_manifest() out of rhizome_read_manifest_file() so that the
out-of-manifest report shows the names of the functions where the manifests
were really allocated.

commandline.c

@@ -1146,19 +1146,19 @@ int app_rhizome_add_file(int argc, const char *const *argv, struct command_line_
     return -1;
   /* Create a new manifest that will represent the file.  If a manifest file was supplied, then read
    * it, otherwise create a blank manifest. */
-  rhizome_manifest *m = NULL;
   int manifest_file_supplied = 0;
-  if (manifestpath[0] && access(manifestpath, R_OK) == 0) {
-    if (debug & DEBUG_RHIZOME) DEBUGF("reading manifest from %s", manifestpath);
-    m = rhizome_read_manifest_file(manifestpath, 0, 0); // no verify
-    if (!m)
-      return WHY("Manifest file could not be loaded -- not added to rhizome");
-    manifest_file_supplied = 1;
-  } else {
-    if (debug & DEBUG_RHIZOME) DEBUGF("manifest file %s does not exist", manifestpath);
-    m = rhizome_new_manifest();
+  rhizome_manifest *m = rhizome_new_manifest();
   if (!m)
     return WHY("Manifest struct could not be allocated -- not added to rhizome");
+  if (manifestpath[0] && access(manifestpath, R_OK) == 0) {
+    if (debug & DEBUG_RHIZOME) DEBUGF("reading manifest from %s", manifestpath);
+    if (rhizome_read_manifest_file(m, manifestpath, 0, 0) == -1) { // no verify
+      rhizome_manifest_free(m);
+      return WHY("Manifest file could not be loaded -- not added to rhizome");
+    }
+    manifest_file_supplied = 1;
+  } else {
+    if (debug & DEBUG_RHIZOME) DEBUGF("manifest file %s does not exist -- creating new manifest", manifestpath);
   }
   /* Fill in a few missing manifest fields, to make it easier to use when adding new files:
       - the default service is FILE

rhizome.c

@@ -52,9 +52,13 @@ int rhizome_bundle_import(rhizome_manifest *m_in, rhizome_manifest **m_out,
   /* Read manifest file if no manifest was given */
   rhizome_manifest *m = m_in;
   if (!m_in) {
-    m = rhizome_read_manifest_file(manifestname, 0 /* file not buffer */, RHIZOME_VERIFY);
+    m = rhizome_new_manifest();
     if (!m)
+      return WHY("Out of manifests.");
+    if (rhizome_read_manifest_file(m, manifestname, 0 /* file not buffer */, RHIZOME_VERIFY) == -1) {
+      rhizome_manifest_free(m);
       return WHY("Could not read manifest file.");
+    }
   } else {
     if (debug&DEBUG_RHIZOMESYNC)
       DEBUGF("Importing direct from manifest structure fileHashedP=%d", m->fileHashedP);

rhizome.h

@@ -199,7 +199,7 @@ int rhizome_write_manifest_file(rhizome_manifest *m, const char *filename);
 int rhizome_manifest_sign(rhizome_manifest *m,const char *authoring_sid);
 int rhizome_drop_stored_file(char *id,int maximum_priority);
 int rhizome_manifest_priority(char *id);
-rhizome_manifest *rhizome_read_manifest_file(const char *filename, int bufferPAndSize, int flags);
+int rhizome_read_manifest_file(rhizome_manifest *m, const char *filename, int bufferPAndSize, int flags);
 int rhizome_hash_file(const char *filename,char *hash_out);
 char *rhizome_manifest_get(const rhizome_manifest *m, const char *var, char *out, int maxlen);
 long long  rhizome_manifest_get_ll(rhizome_manifest *m, const char *var);

rhizome_bundle.c

@@ -21,25 +21,26 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 #include "rhizome.h"
 #include <stdlib.h>
 
-rhizome_manifest *rhizome_read_manifest_file(const char *filename, int bufferP, int flags)
+int rhizome_read_manifest_file(rhizome_manifest *m, const char *filename, int bufferP, int flags)
 {
-  if (bufferP>MAX_MANIFEST_BYTES) return NULL;
-
-  rhizome_manifest *m = rhizome_new_manifest();
-  if (!m) return NULL;
+  if (bufferP>MAX_MANIFEST_BYTES) return WHY("Buffer too big");
+  if (!m) return WHY("Null manifest");
 
   if (bufferP) {
     m->manifest_bytes=bufferP;
     memcpy(m->manifestdata, filename, m->manifest_bytes);
   } else {
-    FILE *f=fopen(filename,"r");
-    if (!f) {
-      WHYF("Could not open manifest file %s for reading.", filename); 
-      rhizome_manifest_free(m);
-      return NULL;
-    }
-    m->manifest_bytes = fread(m->manifestdata,1,MAX_MANIFEST_BYTES,f);
-    fclose(f);
+    FILE *f = fopen(filename, "r");
+    if (f == NULL)
+      return WHYF("Could not open manifest file %s for reading.", filename); 
+    m->manifest_bytes = fread(m->manifestdata, 1, MAX_MANIFEST_BYTES, f);
+    int ret = 0;
+    if (m->manifest_bytes == -1)
+      ret = WHY_perror("fread");
+    if (fclose(f) == EOF)
+      ret = WHY_perror("fclose");
+    if (ret == -1)
+      return -1;
   }
 
   m->manifest_all_bytes=m->manifest_bytes;
@@ -148,7 +149,7 @@ rhizome_manifest *rhizome_read_manifest_file(const char *filename, int bufferP,
   
   m->manifest_bytes=end_of_text;
 
-  return m;
+  return 0;
 }
 
 int rhizome_strn_is_file_hash(const char *text)
@@ -309,9 +310,9 @@ rhizome_manifest *_rhizome_new_manifest(const char *filename, const char *funcna
     {
       int i;
       logMessage(LOG_LEVEL_ERROR, filename, line, funcname, "%s(): no free manifest records, this probably indicates a memory leak", __FUNCTION__);
-      WHYF("   Manifest Slot# | Last allocated by");
+      WHYF("   Slot# | Last allocated by");
       for(i=0;i<MAX_RHIZOME_MANIFESTS;i++) {
-	WHYF("   %-14d | %s:%d in %s()",
+	WHYF("   %-5d | %s:%d in %s()",
 		i,
 		manifest_alloc_sourcefiles[i],
 		manifest_alloc_lines[i],

rhizome_database.c

@@ -617,11 +617,19 @@ int rhizome_list_manifests(const char *service, const char *sender_sid, const ch
 	ret = WHY("Incorrect statement column");
 	break;
       }
+      rhizome_manifest *m = rhizome_new_manifest();
+      if (m == NULL) {
+	ret = WHY("Out of manifests");
+	break;
+      }
+      const char *q_manifestid = (const char *) sqlite3_column_text(statement, 2);
       const char *manifestblob = (char *) sqlite3_column_blob(statement, 3);
       size_t manifestblobsize = sqlite3_column_bytes(statement, 3); // must call after sqlite3_column_blob()
-      rhizome_manifest *m = rhizome_read_manifest_file(manifestblob, manifestblobsize, 0);
-      const char *blob_service = rhizome_manifest_get(m, "service", NULL, 0);
+      if (rhizome_read_manifest_file(m, manifestblob, manifestblobsize, 0) == -1) {
+	WARNF("MANIFESTS row id=%s has invalid manifest blob -- skipped", q_manifestid);
+      } else {
 	int match = 1;
+	const char *blob_service = rhizome_manifest_get(m, "service", NULL, 0);
 	if (service[0] && !(blob_service && strcasecmp(service, blob_service) == 0))
 	  match = 0;
 	if (match && sender_sid[0]) {
@@ -639,14 +647,15 @@ int rhizome_list_manifests(const char *service, const char *sender_sid, const ch
 	  long long blob_date = rhizome_manifest_get_ll(m, "date");
 	  cli_puts(blob_service ? blob_service : ""); cli_delim(":");
 	  cli_puts((const char *)sqlite3_column_text(statement, 0)); cli_delim(":");
-	cli_puts((const char *)sqlite3_column_text(statement, 2)); cli_delim(":");
+	  cli_puts(q_manifestid); cli_delim(":");
 	  cli_printf("%lld", (long long) sqlite3_column_int64(statement, 4)); cli_delim(":");
 	  cli_printf("%lld", (long long) sqlite3_column_int64(statement, 5)); cli_delim(":");
 	  cli_printf("%u", sqlite3_column_int(statement, 1)); cli_delim(":");
 	  cli_printf("%lld", blob_date); cli_delim(":");
 	  cli_puts(blob_name ? blob_name : ""); cli_delim("\n");
 	}
-      rhizome_manifest_free(m);
+      }
+      if (m) rhizome_manifest_free(m);
     }
   }
   sqlite3_finalize(statement);
@@ -919,7 +928,14 @@ int rhizome_find_duplicate(const rhizome_manifest *m, rhizome_manifest **found)
       const char *manifestblob = (char *) sqlite3_column_blob(statement, 1);
       size_t manifestblobsize = sqlite3_column_bytes(statement, 1); // must call after sqlite3_column_blob()
       long long q_version = sqlite3_column_int64(statement, 2);
-      rhizome_manifest *blob_m = rhizome_read_manifest_file(manifestblob, manifestblobsize, 0);
+      rhizome_manifest *blob_m = rhizome_new_manifest();
+      if (blob_m == NULL) {
+	ret = WHY("Out of manifests");
+	break;
+      }
+      if (rhizome_read_manifest_file(blob_m, manifestblob, manifestblobsize, 0) == -1) {
+	WARNF("MANIFESTS row id=%s has invalid manifest blob -- skipped", q_manifestid);
+      } else {
 	const char *blob_service = rhizome_manifest_get(blob_m, "service", NULL, 0);
 	const char *blob_id = rhizome_manifest_get(blob_m, "id", NULL, 0);
 	long long blob_version = rhizome_manifest_get_ll(blob_m, "version");
@@ -987,7 +1003,8 @@ int rhizome_find_duplicate(const rhizome_manifest *m, rhizome_manifest **found)
 	    break;
 	  }
 	}
-      rhizome_manifest_free(blob_m);
+      }
+      if (blob_m) rhizome_manifest_free(blob_m);
     }
   }
   sqlite3_finalize(statement);
@@ -1030,11 +1047,16 @@ int rhizome_retrieve_manifest(const char *manifestid, rhizome_manifest **mp)
 	ret = WHY("Incorrect statement column");
 	break;
       }
+      const char *q_manifestid = (const char *) sqlite3_column_text(statement, 0);
       const char *manifestblob = (char *) sqlite3_column_blob(statement, 1);
       size_t manifestblobsize = sqlite3_column_bytes(statement, 1); // must call after sqlite3_column_blob()
       if (mp) {
-	m = rhizome_read_manifest_file(manifestblob, manifestblobsize, 0);
+	m = rhizome_new_manifest();
 	if (m == NULL) {
+	  WARNF("MANIFESTS row id=%s has invalid manifest blob -- skipped", q_manifestid);
+	  ret = WHY("Out of manifests");
+	} else if (rhizome_read_manifest_file(m, manifestblob, manifestblobsize, 0) == -1) {
+	  WARNF("MANIFESTS row id=%s has invalid manifest blob -- skipped", q_manifestid);
 	  ret = WHY("Invalid manifest blob from database");
 	} else {
 	  ret = 1;
@@ -1063,7 +1085,7 @@ int rhizome_retrieve_manifest(const char *manifestid, rhizome_manifest **mp)
 	    cli_puts("service"); cli_delim(":");
 	    cli_puts(blob_service); cli_delim("\n");
 	    cli_puts("manifestid"); cli_delim(":");
-	    cli_puts((const char *)sqlite3_column_text(statement, 0)); cli_delim("\n");
+	    cli_puts(q_manifestid); cli_delim("\n");
 	    cli_puts("version"); cli_delim(":");
 	    cli_printf("%lld", (long long) sqlite3_column_int64(statement, 2)); cli_delim("\n");
 	    cli_puts("inserttime"); cli_delim(":");

rhizome_packetformats.c

@@ -341,12 +341,16 @@ int overlay_rhizome_saw_advertisements(int i,overlay_frame *f, long long now)
 	   otherwise happen. 
 	   But we do need to make sure that at least one signature is there.
 	*/	
-	m=rhizome_read_manifest_file((char *)&f->payload->bytes[ofs],
-				     manifest_length,RHIZOME_DONTVERIFY);
+	m = rhizome_new_manifest();
 	if (!m) {
 	  WHY("Out of manifests");
 	  return 0;
 	}
+	if (rhizome_read_manifest_file(m, (char *)&f->payload->bytes[ofs], manifest_length, RHIZOME_DONTVERIFY) == -1) {
+	  WHY("Error importing manifest body");
+	  rhizome_manifest_free(m);
+	  return 0;
+	}
 	/* Crude signature presence test */
 	for(i=m->manifest_all_bytes-1;i>0;i--)
 	  if (!m->manifestdata[i]) {
@@ -404,23 +408,25 @@ int overlay_rhizome_saw_advertisements(int i,overlay_frame *f, long long now)
 	  /* Okay, so the manifest looks like it is potentially interesting to us,
 	     i.e., we don't already have it or a later version of it.
 	     Now reread the manifest, this time verifying signatures */
-	  m=rhizome_read_manifest_file((char *)&f->payload->bytes[ofs],
-				       manifest_length,RHIZOME_VERIFY);
-	  if (m->errors) {
+	  if ((m = rhizome_new_manifest()) == NULL)
+	    WHY("Out of manifests");
+	  else if (rhizome_read_manifest_file(m, (char *)&f->payload->bytes[ofs], manifest_length, RHIZOME_VERIFY) == -1) {
+	    WHY("Error importing manifest body");
+	    rhizome_manifest_free(m);	  
+	    m = NULL;
+	  } else if (m->errors) {
 	    if (debug&DEBUG_RHIZOME) DEBUGF("Verifying manifest %s revealed errors -- not storing.", manifest_id);
 	    rhizome_queue_ignore_manifest(m,(struct sockaddr_in*)f->recvaddr,60000);
 	    rhizome_manifest_free(m);	  
+	    m = NULL;
 	  } else {
 	    if (debug&DEBUG_RHIZOME) DEBUGF("Verifying manifest %s revealed no errors -- will try to store.", manifest_id);
-	    
 	    /* Add manifest to import queue. We need to know originating IPv4 address
 	       so that we can transfer by HTTP. */
 	    if (0) DEBUG("Suggesting fetching of a bundle");
 	    rhizome_suggest_queue_manifest_import(m,(struct sockaddr_in *)f->recvaddr);	  
 	  }
 	}
-	else
-	  rhizome_manifest_free(m);	  
 	
 	if (!manifest_length) {
 	  WHY("Infinite loop in packet decoding");