Fix a segfault when log file isn't set and conf is reloaded.

When vmsvc.handler=file, but vmsvc.data isn't set, vmtoolsd crashed
on reloading the config file. This was caused by using a NULL value
as an argument to a strcmp().

This change fixes this by using g_strcmp0 which handles NULL pointers
gracefully, and setting confData to a default value before calling
g_strcmp0().

This also fixes a case where a change in the log file would be ignored.

diff --git a/open-vm-tools/libvmtools/vmtoolsLog.c b/open-vm-tools/libvmtools/vmtoolsLog.c
index 8aef0bdd3b4817428fd2f98210ddb4ce127f379e..74312db31119e375e7d4965476d38c86554317f7 100644 (file)
--- a/open-vm-tools/libvmtools/vmtoolsLog.c
+++ b/open-vm-tools/libvmtools/vmtoolsLog.c
@@ -521,6 +521,38 @@ exit:
 }
 
 
+/*
+ *******************************************************************************
+ * VMToolsDefaultLogFilePath --                                               */ /**
+ *
+ * @brief Creates a default log file path
+ *
+ * @param[in] domain    Domain name.
+ *
+ * @return Default path for the log file. Caller is responsibe to g_free() it.
+ *
+ *******************************************************************************
+ */
+
+static gchar *
+VMToolsDefaultLogFilePath(const gchar *domain)
+{
+   gchar *path;
+#ifdef WIN32
+   gchar winDir[MAX_PATH];
+
+   Win32U_ExpandEnvironmentStrings(DEFAULT_LOGFILE_DIR,
+                                   (LPSTR) winDir, sizeof winDir);
+   path = g_strdup_printf("%s%sTemp%s%s-%s.log",
+                          winDir, DIRSEPS, DIRSEPS,
+                          DEFAULT_LOGFILE_NAME_PREFIX, domain);
+#else
+   path = g_strdup_printf("%s-%s.log", DEFAULT_LOGFILE_NAME_PREFIX, domain);
+#endif
+   return path;
+}
+
+
 /*
  *******************************************************************************
  * VMToolsGetLogFilePath --                                               */ /**
@@ -547,18 +579,7 @@ VMToolsGetLogFilePath(const gchar *key,
 
    path = g_key_file_get_string(cfg, LOGGING_GROUP, key, NULL);
    if (path == NULL) {
-#ifdef WIN32
-      gchar winDir[MAX_PATH];
-
-      Win32U_ExpandEnvironmentStrings(DEFAULT_LOGFILE_DIR,
-                                      (LPSTR) winDir, sizeof winDir);
-      path = g_strdup_printf("%s%sTemp%s%s-%s.log",
-                             winDir, DIRSEPS, DIRSEPS,
-                             DEFAULT_LOGFILE_NAME_PREFIX, domain);
-#else
-      path = g_strdup_printf("%s-%s.log", DEFAULT_LOGFILE_NAME_PREFIX, domain);
-#endif
-      return path;
+      return VMToolsDefaultLogFilePath(domain);
    }
 
    len = strlen(path);
@@ -777,6 +798,12 @@ VMToolsConfigLogDomain(const gchar *domain,
       handler = g_strdup(DEFAULT_HANDLER);
    }
 
+   if (confData == NULL) {
+      if ((g_strcmp0(handler, "file") == 0) || (g_strcmp0(handler, "file+") == 0)) {
+         confData = VMToolsDefaultLogFilePath(domain);
+      }
+   }
+
    /* Parse the log level configuration, and build the mask. */
    if (strcmp(level, "error") == 0) {
       levelsMask = G_LOG_LEVEL_ERROR;
@@ -817,13 +844,13 @@ VMToolsConfigLogDomain(const gchar *domain,
       const char *oldtype = oldDefault != NULL ? oldDefault->type : NULL;
       const char *oldData = oldDefault != NULL ? oldDefault->confData : NULL;
 
-      if (oldtype != NULL && strcmp(oldtype, "file+") == 0) {
+      if (g_strcmp0(oldtype, "file+") == 0) {
          oldtype = "file";
       }
 
-      if (isDefault && oldtype != NULL && strcmp(oldtype, handler) == 0) {
-         // check for a filename change
-         if (oldData && strcmp(oldData, confData) == 0) {
+      if (isDefault && g_strcmp0(oldtype, handler) == 0) {
+         /* check for a filename change */
+         if (g_strcmp0(oldData, confData) == 0) {
             data = oldDefault;
          }
       } else if (oldDomains != NULL) {
@@ -831,7 +858,8 @@ VMToolsConfigLogDomain(const gchar *domain,
          for (i = 0; i < oldDomains->len; i++) {
             LogHandler *old = g_ptr_array_index(oldDomains, i);
             if (old != NULL && !old->inherited && strcmp(old->domain, domain) == 0) {
-               if (strcmp(old->type, handler) == 0) {
+               if (g_strcmp0(old->type, handler) == 0 &&
+                   g_strcmp0(old->confData, confData) == 0) {
                   data = old;
                   oldDomains->pdata[i] = NULL;
                }