logstorage: Fix issue with gzip (#571)

Current implementation facing issue generating gzip dlt log, hence fix is applied: + Fsync for gzip + Adapt option parsing of gzip with others + Align gzip otpion status (on,off,default,unknown) + Make gzip a dlt proprietary Signed-off-by: LUU QUANG MINH <[email protected]> Co-authored-by: LUU QUANG MINH <[email protected]>
COVESA · Dec 15, 2023 · 543087b · 543087b
1 parent 2118762
commit 543087b
Show file tree

Hide file tree

Showing 7 changed files with 63 additions and 111 deletions.
diff --git a/doc/dlt_offline_logstorage.md b/doc/dlt_offline_logstorage.md
@@ -77,7 +77,7 @@ NOFiles=<number of files>            # Number of created files before oldest is
 SyncBehavior=<strategy>              # Specify sync strategy. Default: Sync'ed after every message. See Logstorage Ringbuffer Implementation below.
 EcuID=<ECUid>                        # Specify ECU identifier
 SpecificSize=<spec size in bytes>    # Store logs in storage devices after specific size is reached.
-GzipCompression=<on or off>          # Write the logfiles with gzip compression.
+GzipCompression=<ON/OFF>             # Write the logfiles with gzip compression.
 OverwriteBehavior=<strategy>         # Specify overwrite strategy. Default: Delete oldest file and continue. See Logstorage Ringbuffer Implementation below.
 DisableNetwork=<ON/OFF>              # Specify if the message shall be routed to network client.
 ```

diff --git a/src/offlinelogstorage/dlt_offline_logstorage.c b/src/offlinelogstorage/dlt_offline_logstorage.c
@@ -457,34 +457,6 @@ DLT_STATIC int dlt_logstorage_read_number(unsigned int *number, char *value)
     return dlt_logstorage_set_number(number, size);
 }
 
-/**
- * dlt_logstorage_read_bool
- *
- * Evaluate a boolean config value. Values such as '1', 'on' or 'true' will be
- * treated as true otherwise the config value will be interpreted as false.
- *
- * @param bool     The boolean to populate
- * @param value    The string from the config file
- * @returns        0 on success, -1 on error
- */
-DLT_STATIC int dlt_logstorage_read_bool(unsigned int *boolean, char *value)
-{
-    int len = 0;
-    if (value == NULL)
-        return -1;
-
-    len = strnlen(value, 5);
-    *boolean = 0;
-    if (strncmp(value, "on", len) == 0) {
-        *boolean = 1;
-    } else if (strncmp(value, "true", len) == 0) {
-        *boolean = 1;
-    } else if (strncmp(value, "1", len) == 0) {
-        *boolean = 1;
-    }
-    return 0;
-}
-
 /**
  * dlt_logstorage_get_keys_list
  *
@@ -1206,20 +1178,6 @@ DLT_STATIC int dlt_logstorage_check_nofiles(DltLogStorageFilterConfig *config,
     return dlt_logstorage_read_number(&config->num_files, value);
 }
 
-DLT_STATIC int dlt_logstorage_check_gzip_compression(DltLogStorageFilterConfig *config,
-                                                     char *value)
-{
-    if ((config == NULL) || (value == NULL))
-        return -1;
-
-    int result = dlt_logstorage_read_bool(&config->gzip_compression, value);
-#ifndef DLT_LOGSTORAGE_USE_GZIP
-    dlt_log(LOG_WARNING, "dlt-daemon not compiled with logstorage gzip support\n");
-    config->gzip_compression = 0;
-#endif
-    return result;
-}
-
 DLT_STATIC int dlt_logstorage_check_specificsize(DltLogStorageFilterConfig *config,
                                                  char *value)
 {
@@ -1359,6 +1317,42 @@ DLT_STATIC int dlt_logstorage_check_disable_network(DltLogStorageFilterConfig *c
     return 0;
 }
 
+/**
+ * dlt_logstorage_check_gzip_compression
+ *
+ * Evaluate gzip compression. The gzip compression is an optional filter
+ * configuration parameter.
+ * If the given value cannot be associated with a flag, the default
+ * flag will be assigned.
+ *
+ * @param[in] config    DltLogStorageFilterConfig
+ * @param[in] value     string given in config file
+ * @return              0 on success, 1 on unknown value, -1 on error
+ */
+DLT_STATIC int dlt_logstorage_check_gzip_compression(DltLogStorageFilterConfig *config,
+                                                     char *value)
+{
+#ifdef DLT_LOGSTORAGE_USE_GZIP
+    if ((config == NULL) || (value == NULL))
+        return -1;
+
+    if (strcasestr(value, "ON") != NULL) {
+        config->gzip_compression = DLT_LOGSTORAGE_GZIP_ON;
+    } else if (strcasestr(value, "OFF") != NULL) {
+        config->gzip_compression = DLT_LOGSTORAGE_GZIP_OFF;
+    } else {
+        dlt_log(LOG_WARNING,
+                "Unknown gzip compression flag. Set default OFF\n");
+        config->gzip_compression = DLT_LOGSTORAGE_GZIP_OFF;
+        return 1;
+    }
+#else
+    dlt_log(LOG_WARNING, "dlt-daemon not compiled with logstorage gzip support\n");
+    config->gzip_compression = 0;
+#endif
+    return 0;
+}
+
 /**
  * dlt_logstorage_check_ecuid
  *

diff --git a/src/offlinelogstorage/dlt_offline_logstorage.h b/src/offlinelogstorage/dlt_offline_logstorage.h
@@ -112,17 +112,23 @@
 #define DLT_OFFLINE_LOGSTORAGE_IS_STRATEGY_SET(S, s) ((S)&(s))
 
 /* Offline Logstorage overwrite strategies */
-#define DLT_LOGSTORAGE_OVERWRITE_ERROR         -1 /* error case */
-#define DLT_LOGSTORAGE_OVERWRITE_UNSET          0 /* strategy not set */
-#define DLT_LOGSTORAGE_OVERWRITE_DISCARD_OLD    1 /* default, discard old */
-#define DLT_LOGSTORAGE_OVERWRITE_DISCARD_NEW   (1 << 1) /* discard new */
+#define DLT_LOGSTORAGE_OVERWRITE_ERROR          -1 /* error case */
+#define DLT_LOGSTORAGE_OVERWRITE_UNSET           0 /* strategy not set */
+#define DLT_LOGSTORAGE_OVERWRITE_DISCARD_OLD     1 /* default, discard old */
+#define DLT_LOGSTORAGE_OVERWRITE_DISCARD_NEW    (1 << 1) /* discard new */
 
 /* Offline Logstorage disable network routing */
 #define DLT_LOGSTORAGE_DISABLE_NW_ERROR         -1 /* error case */
 #define DLT_LOGSTORAGE_DISABLE_NW_UNSET          0 /* not set */
 #define DLT_LOGSTORAGE_DISABLE_NW_OFF            1 /* default, enable network routing */
 #define DLT_LOGSTORAGE_DISABLE_NW_ON            (1 << 1) /* disable network routing */
 
+/* Offline Logstorage disable network routing */
+#define DLT_LOGSTORAGE_GZIP_ERROR               -1 /* error case */
+#define DLT_LOGSTORAGE_GZIP_UNSET                0 /* not set */
+#define DLT_LOGSTORAGE_GZIP_OFF                  1 /* default, enable network routing */
+#define DLT_LOGSTORAGE_GZIP_ON                  (1 << 1) /* disable network routing */
+
 /* logstorage max cache */
 extern unsigned int g_logstorage_cache_max;
 /* current logstorage cache size */
@@ -208,9 +214,7 @@ struct DltLogStorageFilterConfig
                                int status);
     FILE *log;                      /* current open log file */
     int fd;                         /* The file descriptor for the active log file */
-#ifdef DLT_LOGSTORAGE_USE_GZIP
-    gzFile gzlog;                   /* current open gz log file */
-#endif
+    gzFile *gzlog;                  /* current open gz log file */
     void *cache;                    /* log data cache */
     unsigned int specific_size;     /* cache size used for specific_size sync strategy */
     unsigned int current_write_file_offset;    /* file offset for specific_size sync strategy */

diff --git a/src/offlinelogstorage/dlt_offline_logstorage_behavior.c b/src/offlinelogstorage/dlt_offline_logstorage_behavior.c
@@ -479,7 +479,7 @@ int dlt_logstorage_storage_dir_info(DltLogStorageUserConfig *file_config,
 }
 
 /**
- * dlt_logstorage_open_log_file
+ * dlt_logstorage_open_log_output_file
  *
  * Open a handle to the logfile
  *
@@ -581,7 +581,7 @@ int dlt_logstorage_open_log_file(DltLogStorageFilterConfig *config,
         strcat(absolute_file_path, storage_path);
         strcat(absolute_file_path, file_name);
         config->working_file_name = strdup(file_name);
-        dlt_logstorage_open_log_output_file(config, absolute_file_path, "a+");
+        dlt_logstorage_open_log_output_file(config, absolute_file_path, "a");
 
         /* Add file to file list */
         *tmp = malloc(sizeof(DltLogStorageFileList));
@@ -624,7 +624,7 @@ int dlt_logstorage_open_log_file(DltLogStorageFilterConfig *config,
         if ((ret == 0) &&
             ((is_sync && (s.st_size < (int)config->file_size)) ||
              (!is_sync && (s.st_size + msg_size <= (int)config->file_size)))) {
-            dlt_logstorage_open_log_output_file(config, absolute_file_path, "a+");
+            dlt_logstorage_open_log_output_file(config, absolute_file_path, "a");
             config->current_write_file_offset = s.st_size;
         }
         else {
@@ -1105,9 +1105,18 @@ int dlt_logstorage_prepare_on_msg(DltLogStorageFilterConfig *config,
                 /* Sync only if on_msg */
                 if ((config->sync == DLT_LOGSTORAGE_SYNC_ON_MSG) ||
                     (config->sync == DLT_LOGSTORAGE_SYNC_UNSET)) {
-                    if (fsync(fileno(config->log)) != 0) {
-                        if (errno != ENOSYS) {
-                            dlt_vlog(LOG_ERR, "%s: failed to sync log file\n", __func__);
+                    if (config->gzip_compression) {
+                        if (fsync(fileno(config->gzlog)) != 0) {
+                            if (errno != ENOSYS) {
+                                dlt_vlog(LOG_ERR, "%s: failed to sync gzip log file\n", __func__);
+                            }
+                        }
+                    }
+                    else {
+                        if (fsync(fileno(config->log)) != 0) {
+                            if (errno != ENOSYS) {
+                                dlt_vlog(LOG_ERR, "%s: failed to sync log file\n", __func__);
+                            }
                         }
                     }
                 }

diff --git a/src/offlinelogstorage/dlt_offline_logstorage_internal.h b/src/offlinelogstorage/dlt_offline_logstorage_internal.h
@@ -68,8 +68,6 @@ DLT_STATIC int dlt_logstorage_count_ids(const char *str);
 
 DLT_STATIC int dlt_logstorage_read_number(unsigned int *number, char *value);
 
-DLT_STATIC int dlt_logstorage_read_bool(unsigned int *boolean, char *value);
-
 DLT_STATIC int dlt_logstorage_read_list_of_names(char **names, const char *value);
 
 DLT_STATIC int dlt_logstorage_check_apids(DltLogStorageFilterConfig *config, char *value);

diff --git a/tests/components/logstorage/logstorage_one_file/dlt.conf.in b/tests/components/logstorage/logstorage_one_file/dlt.conf.in
@@ -1,7 +1,5 @@
 ControlSocketPath = ./dlt-ctrl.sock
 
-ControlSocketPath = ./dlt-ctrl.sock
-
 OfflineLogstorageTimestamp = 0
 OfflineLogstorageMaxDevices = 1
 OfflineLogstorageDirPath = .

diff --git a/tests/gtest_dlt_daemon_offline_log.cpp b/tests/gtest_dlt_daemon_offline_log.cpp
@@ -181,57 +181,6 @@ TEST(t_dlt_logstorage_read_number, null)
     EXPECT_EQ(DLT_RETURN_ERROR, dlt_logstorage_read_number(&number, NULL));
 }
 
-TEST(t_dlt_logstorage_read_boolean, normal)
-{
-    unsigned int val;
-    {
-        char str[] = "0";
-        EXPECT_EQ(DLT_RETURN_OK, dlt_logstorage_read_bool(&val, str));
-        EXPECT_EQ(0, val);
-    }
-    {
-        char str[] = "1";
-        EXPECT_EQ(DLT_RETURN_OK, dlt_logstorage_read_bool(&val, str));
-        EXPECT_EQ(1, val);
-    }
-    {
-        char str[] = "off";
-        EXPECT_EQ(DLT_RETURN_OK, dlt_logstorage_read_bool(&val, str));
-        EXPECT_EQ(0, val);
-    }
-    {
-        char str[] = "on";
-        EXPECT_EQ(DLT_RETURN_OK, dlt_logstorage_read_bool(&val, str));
-        EXPECT_EQ(1, val);
-    }
-    {
-        char str[] = "false";
-        EXPECT_EQ(DLT_RETURN_OK, dlt_logstorage_read_bool(&val, str));
-        EXPECT_EQ(0, val);
-    }
-    {
-        char str[] = "true";
-        EXPECT_EQ(DLT_RETURN_OK, dlt_logstorage_read_bool(&val, str));
-        EXPECT_EQ(1, val);
-    }
-    {
-        char str[] = "invalidvalue";
-        EXPECT_EQ(DLT_RETURN_OK, dlt_logstorage_read_bool(&val, str));
-        EXPECT_EQ(0, val);
-    }
-    {
-        char str[] = "not";
-        EXPECT_EQ(DLT_RETURN_OK, dlt_logstorage_read_bool(&val, str));
-        EXPECT_EQ(0, val);
-    }
-}
-
-TEST(t_dlt_logstorage_read_boolean, null)
-{
-    unsigned int val;
-    EXPECT_EQ(DLT_RETURN_ERROR, dlt_logstorage_read_bool(&val, NULL));
-}
-
 /* Begin Method: dlt_logstorage::t_dlt_logstorage_create_keys*/
 TEST(t_dlt_logstorage_create_keys, normal)
 {