From 55c79e9c7e14b501baa72bc8b415e0a66752ed01 Mon Sep 17 00:00:00 2001
From: Dave Barach <dave@barachs.net>
Date: Tue, 5 Dec 2017 14:48:56 -0500
Subject: [PATCH] make clib_maplog_update_header(...) globally accessible

clib_maplog_process(...): handle logs which weren't closed properly.
It will happen.

Change-Id: Ibcf9c9ea7a09991e6294050e7d2979a0d3f965cf
Signed-off-by: Dave Barach <dave@barachs.net>
---
 src/vppinfra/maplog.c      | 63 +++++++++++++++++++++++++++++++---------------
 src/vppinfra/maplog.h      |  1 +
 src/vppinfra/test_maplog.c | 14 +++++++++--
 3 files changed, 56 insertions(+), 22 deletions(-)

diff --git a/src/vppinfra/maplog.c b/src/vppinfra/maplog.c
index 41ee0757897..3e08973bc8c 100644
--- a/src/vppinfra/maplog.c
+++ b/src/vppinfra/maplog.c
@@ -224,35 +224,20 @@ _clib_maplog_get_entry_slowpath (clib_maplog_main_t * mm, u64 my_record_index)
 }
 
 /**
- * @brief Close a mapped log, and update the log header file
+ * @brief Update a mapped log header file
  *
- * Unmap the current log segments.
  * Read the log header. Update the number of records, and number of files
- *
  * @param[in/out] mm	mapped log object
  */
 void
-clib_maplog_close (clib_maplog_main_t * mm)
+clib_maplog_update_header (clib_maplog_main_t * mm)
 {
-  int i, rv;
-  u64 file_size_in_bytes;
-  int fd;
+  int fd, rv;
   clib_maplog_header_t _h, *h = &_h;
 
   if (!(mm->flags & CLIB_MAPLOG_FLAG_INIT))
     return;
 
-  file_size_in_bytes =
-    mm->file_size_in_records * mm->record_size_in_cachelines *
-    CLIB_CACHE_LINE_BYTES;
-
-  /* unmap current + next segments */
-  for (i = 0; i < 2; i++)
-    {
-      (void) munmap ((u8 *) mm->file_baseva[i], file_size_in_bytes);
-      vec_free (mm->filenames[i]);
-    }
-
   /* Open the log header */
   fd = open ((char *) mm->header_filename, O_RDWR, 0600);
   if (fd < 0)
@@ -286,6 +271,37 @@ clib_maplog_close (clib_maplog_main_t * mm)
 out:
   if (fd >= 0)
     (void) close (fd);
+}
+
+/**
+ * @brief Close a mapped log, and update the log header file
+ *
+ * Unmap the current log segments.
+ * Read the log header. Update the number of records, and number of files
+ *
+ * @param[in/out] mm	mapped log object
+ */
+void
+clib_maplog_close (clib_maplog_main_t * mm)
+{
+  int i;
+  u64 file_size_in_bytes;
+
+  if (!(mm->flags & CLIB_MAPLOG_FLAG_INIT))
+    return;
+
+  clib_maplog_update_header (mm);
+
+  file_size_in_bytes =
+    mm->file_size_in_records * mm->record_size_in_cachelines *
+    CLIB_CACHE_LINE_BYTES;
+
+  /* unmap current + next segments */
+  for (i = 0; i < 2; i++)
+    {
+      (void) munmap ((u8 *) mm->file_baseva[i], file_size_in_bytes);
+      vec_free (mm->filenames[i]);
+    }
 
   vec_free (mm->file_basename);
   vec_free (mm->header_filename);
@@ -335,6 +351,14 @@ brief:
  * Reads the maplog header. Map and process all log segments in order.
  * Calls the callback function once per file with a record count.
  *
+ * Note: if the file header isn't updated by calling
+ * clib_maplog_close(), it will appear to have an infinite
+ * number of records in an infinite number of files.
+ *
+ * So long as the callback function understands that possibility
+ * - by simply ignoring NULL records - the scheme still
+ * works...
+ *
  * @param [in] file_basename Same basename supplied to clib_maplog_init
  * @param [in] fp_arg Callback function pointer
  */
@@ -347,7 +371,7 @@ clib_maplog_process (char *file_basename, void *fp_arg)
   u64 file_size_in_bytes;
   u8 *header_filename, *this_filename = 0;
   u8 *file_baseva;
-  void (*fp) (clib_maplog_header_t *, void *data, u64 count);
+  int (*fp) (clib_maplog_header_t *, void *data, u64 count);
   u64 records_this_file, records_left;
   ASSERT (fp_arg);
 
@@ -384,7 +408,6 @@ clib_maplog_process (char *file_basename, void *fp_arg)
       fd = open ((char *) this_filename, O_RDONLY, 0600);
       if (fd < 0)
 	{
-	  clib_unix_warning ("open maplog file");
 	  rv = -3;
 	  goto out;
 	}
diff --git a/src/vppinfra/maplog.h b/src/vppinfra/maplog.h
index d1c23d20d29..9bc8596de1b 100644
--- a/src/vppinfra/maplog.h
+++ b/src/vppinfra/maplog.h
@@ -107,6 +107,7 @@ typedef struct
 /* function prototypes */
 
 int clib_maplog_init (clib_maplog_init_args_t * ap);
+void clib_maplog_update_header (clib_maplog_main_t * mm);
 void clib_maplog_close (clib_maplog_main_t * mm);
 int clib_maplog_process (char *file_basename, void *fp_arg);
 
diff --git a/src/vppinfra/test_maplog.c b/src/vppinfra/test_maplog.c
index c49ebd6885e..edb61bd84d0 100644
--- a/src/vppinfra/test_maplog.c
+++ b/src/vppinfra/test_maplog.c
@@ -38,6 +38,9 @@ process_maplog_records (clib_maplog_header_t * h,
 
   while (records_this_file--)
     {
+      /* Padding at the end of a damaged log? */
+      if (e->serial_number == 0ULL)
+	break;
       fformat (stdout, "%4lld ", e->serial_number);
       if (++i == 8)
 	{
@@ -57,6 +60,10 @@ test_maplog_main (unformat_input_t * input)
   int rv;
   int i;
   test_entry_t *t;
+  int noclose = 0;
+
+  if (unformat (input, "noclose"))
+    noclose = 1;
 
   memset (a, 0, sizeof (*a));
   a->mm = mm;
@@ -79,10 +86,13 @@ test_maplog_main (unformat_input_t * input)
   for (i = 0; i < 64 * 5; i++)
     {
       t = clib_maplog_get_entry (mm);
-      t->serial_number = i;
+      t->serial_number = i + 1;
     }
 
-  clib_maplog_close (mm);
+  if (noclose)
+    memset (mm, 0, sizeof (*mm));
+  else
+    clib_maplog_close (mm);
 
   clib_maplog_process ("/tmp/maplog_test", process_maplog_records);
 
-- 
2.16.6