simplify logging a bit. warn_client() is gone, reuse error_msg() for it.

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

9 files changed, 91 insertions, 76 deletions

diff --git a/lib/Plugins/Bugzilla.cpp b/lib/Plugins/Bugzilla.cpp
index ca83d369..18ab5c87 100644
--- a/lib/Plugins/Bugzilla.cpp
+++ b/lib/Plugins/Bugzilla.cpp
@@ -313,7 +313,7 @@ int32_t ctx::check_uuid_in_bugzilla(const char* component, const char* UUID)
             xmlrpc_int bug_id;
             xmlrpc_read_int(&env, bug, &bug_id);
             log("Bug is already reported: %i", (int)bug_id);
-            update_client(_("Bug is already reported: ") + to_string(bug_id));
+            update_client(_("Bug is already reported: %i"), (int)bug_id);
 
             xmlrpc_DECREF(bug);
             xmlrpc_DECREF(item);
@@ -375,7 +375,7 @@ uint32_t ctx::new_bug(const map_crash_report_t& pCrashReport)
         xmlrpc_read_int(&env, id, &bug_id);
         throw_if_xml_fault_occurred(&env);
         log("New bug id: %i", bug_id);
-        update_client(_("New bug id: ") + to_string(bug_id));
+        update_client(_("New bug id: %i"), bug_id);
     }
 
     xmlrpc_DECREF(result);
diff --git a/lib/Plugins/CCpp.cpp b/lib/Plugins/CCpp.cpp
index a2d65f36..b6a5db85 100644
--- a/lib/Plugins/CCpp.cpp
+++ b/lib/Plugins/CCpp.cpp
@@ -610,8 +610,8 @@ Another application is holding the yum lock, cannot continue
         if (last >= 0 && buff[last] == '\n')
             buff[last] = '\0';
 
-        /* log(buff); - update_client logs it too */
-        update_client(buff); /* maybe only if buff != ""? */
+        log("%s", buff);
+        update_client("%s", buff); /* maybe only if buff != ""? */
 
 #ifdef COMPLAIN_IF_NO_DEBUGINFO
         if (already_installed == false)
@@ -715,8 +715,8 @@ static void InstallDebugInfos(const char *pDebugDumpDir, std::string& build_ids)
         }
         if (*p)
         {
-            /* log(buff); - update_client logs it too */
-            update_client(buff);
+            log("%s", buff);
+            update_client("%s", buff);
         }
     }
 
diff --git a/lib/Plugins/Catcut.cpp b/lib/Plugins/Catcut.cpp
index 6bb44a9d..23b1d2c4 100644
--- a/lib/Plugins/Catcut.cpp
+++ b/lib/Plugins/Catcut.cpp
@@ -162,9 +162,7 @@ static void create_new_bug_description(const map_crash_report_t& pCrashReport, s
         }
         else if (it->second[CD_TYPE] == CD_BIN)
         {
-            string msg = ssprintf(_("Binary file %s will not be reported."), it->first.c_str());
-            warn_client(msg);
-            //update_client(_("Binary file ")+it->first+_(" will not be reported."));
+            error_msg(_("Binary file %s will not be reported"), it->first.c_str());
         }
     }
 }
@@ -244,7 +242,7 @@ static string new_bug(const char *auth_cookie, const map_crash_report_t& pCrashR
     throw_if_xml_fault_occurred();
     bug_id_str = bug_id;
     log("New bug id: %s", bug_id);
-    update_client(_("New bug id: ") + bug_id_str);
+    update_client(_("New bug id: %s"), bug_id);
     free((void*)bug_id);
     xmlrpc_DECREF(bug_id_xml);
 
diff --git a/lib/Plugins/FileTransfer.cpp b/lib/Plugins/FileTransfer.cpp
index 8a88fefd..2220e940 100644
--- a/lib/Plugins/FileTransfer.cpp
+++ b/lib/Plugins/FileTransfer.cpp
@@ -60,12 +60,11 @@ void CFileTransfer::SendFile(const char *pURL, const char *pFilename)
     int len = strlen(pURL);
     if (len == 0)
     {
-        warn_client(_("FileTransfer: URL not specified"));
+        error_msg(_("FileTransfer: URL not specified"));
         return;
     }
 
-    string msg = ssprintf(_("Sending archive %s to %s"), pFilename, pURL);
-    update_client(msg.c_str());
+    update_client(_("Sending archive %s to %s"), pFilename, pURL);
 
     string wholeURL = concat_path_file(pURL, pFilename);
 
@@ -332,8 +331,7 @@ void CFileTransfer::Run(const char *pActionDir, const char *pArgs)
         }
         catch (CABRTException& e)
         {
-            warn_client(ssprintf(_("Can't create and send an archive: %s"), e.what()));
-            //update_client("CFileTransfer::Run(): Cannot create and send an archive: " + e.what());
+            error_msg(_("Can't create and send an archive: %s"), e.what());
         }
         unlink(archivename.c_str());
     }
@@ -358,8 +356,7 @@ void CFileTransfer::Run(const char *pActionDir, const char *pArgs)
             }
             catch (CABRTException& e)
             {
-                warn_client(ssprintf(_("Can't create and send an archive: "), e.what()));
-//                update_client("CFileTransfer::Run(): Cannot create and send an archive: " + e.what());
+                error_msg(_("Can't create and send an archive %s"), e.what());
             }
             unlink(archivename.c_str());
         }
@@ -381,7 +378,7 @@ void CFileTransfer::SetSettings(const map_plugin_settings_t& pSettings)
     }
     else
     {
-        warn_client(_("FileTransfer: URL not specified"));
+        error_msg(_("FileTransfer: URL not specified"));
     }
 
     it = pSettings.find("RetryCount");
diff --git a/lib/Plugins/SOSreport.cpp b/lib/Plugins/SOSreport.cpp
index da77cce1..fedc51ab 100644
--- a/lib/Plugins/SOSreport.cpp
+++ b/lib/Plugins/SOSreport.cpp
@@ -119,7 +119,7 @@ void CActionSOSreport::Run(const char *pActionDir, const char *pArgs)
         command = ssprintf("%s %s 2>&1", command_prefix, args[0].c_str());
     }
 
-    update_client(_("running sosreport: ") + command);
+    update_client(_("running sosreport: %s"), command.c_str());
     FILE *fp = popen(command.c_str(), "r");
     if (fp == NULL)
     {
diff --git a/lib/Plugins/TicketUploader.cpp b/lib/Plugins/TicketUploader.cpp
index 4aa05b7c..60cba311 100644
--- a/lib/Plugins/TicketUploader.cpp
+++ b/lib/Plugins/TicketUploader.cpp
@@ -43,7 +43,7 @@ CTicketUploader::~CTicketUploader()
 
 static void Error(const char *msg)
 {
-    update_client(msg);
+    update_client("%s", msg);
     throw CABRTException(EXCEP_PLUGIN, msg);
 }
 
@@ -102,11 +102,11 @@ void CTicketUploader::SendFile(const char *pURL, const char *pFilename)
 {
     if (pURL[0] == '\0')
     {
-        warn_client(_("FileTransfer: URL not specified"));
+        error_msg(_("FileTransfer: URL not specified"));
         return;
     }
 
-    update_client(ssprintf(_("Sending archive %s to %s"), pFilename, pURL));
+    update_client(_("Sending archive %s to %s"), pFilename, pURL);
 
     const char *base = (strrchr(pFilename, '/') ? : pFilename-1) + 1;
     string wholeURL = concat_path_file(pURL, base);
@@ -133,7 +133,6 @@ void CTicketUploader::SendFile(const char *pURL, const char *pFilename)
         curl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);
         /* specify target */
         curl_easy_setopt(curl, CURLOPT_URL, wholeURL.c_str());
-        /*file handle: passed to the default callback, it will fread() it*/
         curl_easy_setopt(curl, CURLOPT_READDATA, f);
         curl_easy_setopt(curl, CURLOPT_INFILESIZE, buf.st_size);
         /* everything is done here; result 0 means success */
@@ -143,7 +142,7 @@ void CTicketUploader::SendFile(const char *pURL, const char *pFilename)
         fclose(f);
         if (result != 0)
         {
-            update_client(ssprintf(_("Sending failed, trying again. %s"), curl_easy_strerror((CURLcode)result)));
+            update_client(_("Sending failed, trying again. %s"), curl_easy_strerror((CURLcode)result));
         }
     }
     /*retry the upload if not succesful, wait a bit before next try*/
@@ -317,19 +316,19 @@ string CTicketUploader::Report(const map_crash_report_t& pCrashReport,
     }
     msgbuf << _("END: ") << endl;
 
-    warn_client(msgbuf.str());
+    error_msg("%s", msgbuf.str());
 
     string ret;
     if (do_upload)
     {
         string xx = _("report sent to ") + upload_url + '/' + outfile_basename;
-        update_client(xx);
+        update_client("%s", xx);
         ret = xx;
     }
     else
     {
         string xx = _("report copied to /tmp/") + outfile_basename;
-        update_client(xx);
+        update_client("%s", xx);
         ret = xx;
     }
 
diff --git a/lib/Utils/CommLayerInner.cpp b/lib/Utils/CommLayerInner.cpp
index 8490e2fa..307fe66d 100644
--- a/lib/Utils/CommLayerInner.cpp
+++ b/lib/Utils/CommLayerInner.cpp
@@ -10,29 +10,48 @@ static map_uint_str_t s_mapClientID;
 static pthread_mutex_t s_map_mutex;
 static bool s_map_mutex_inited;
 
+/* called via [p]error_msg() */
+static void warn_client(const char *msg)
+{
+    if (!s_pObs)
+        return;
+
+    uint64_t key = uint64_t(pthread_self());
+
+    pthread_mutex_lock(&s_map_mutex);
+    map_uint_str_t::const_iterator ki = s_mapClientID.find(key);
+    const char* peer = (ki != s_mapClientID.end() ? ki->second.c_str() : NULL);
+    pthread_mutex_unlock(&s_map_mutex);
+
+    if (peer)
+        s_pObs->Warning(msg, peer, key);
+}
+
 void init_daemon_logging(CObserver *pObs)
 {
     s_pObs = pObs;
     if (!s_map_mutex_inited)
     {
-        pthread_mutex_init(&s_map_mutex, NULL);
         s_map_mutex_inited = true;
+        pthread_mutex_init(&s_map_mutex, NULL);
+        g_custom_logger = &warn_client;
     }
 }
 
-void set_client_name(const char* name)
+void set_client_name(const char *name)
 {
     uint64_t key = uint64_t(pthread_self());
 
     pthread_mutex_lock(&s_map_mutex);
-    if (!name)
+    if (!name) {
         s_mapClientID.erase(key);
-    else
+    } else {
         s_mapClientID[key] = name;
+    }
     pthread_mutex_unlock(&s_map_mutex);
 }
 
-void warn_client(const std::string& pMessage)
+void update_client(const char *fmt, ...)
 {
     if (!s_pObs)
         return;
@@ -44,26 +63,16 @@ void warn_client(const std::string& pMessage)
     const char* peer = (ki != s_mapClientID.end() ? ki->second.c_str() : NULL);
     pthread_mutex_unlock(&s_map_mutex);
 
-    if (peer)
-        s_pObs->Warning(pMessage.c_str(), peer, key);
-    else /* Bug: someone tries to warn_client() without set_client_name()!? */
-        log("Hmm, stray %s: '%s'", __func__, pMessage.c_str());
-}
-
-void update_client(const std::string& pMessage)
-{
-    if (!s_pObs)
+    if (!peer)
         return;
 
-    uint64_t key = uint64_t(pthread_self());
+    va_list p;
+    va_start(p, fmt);
+    char *msg;
+    int used = vasprintf(&msg, fmt, p);
+    va_end(p);
+    if (used < 0)
+	return;
 
-    pthread_mutex_lock(&s_map_mutex);
-    map_uint_str_t::const_iterator ki = s_mapClientID.find(key);
-    const char* peer = (ki != s_mapClientID.end() ? ki->second.c_str() : NULL);
-    pthread_mutex_unlock(&s_map_mutex);
-
-    if (peer)
-        s_pObs->Status(pMessage.c_str(), peer, key);
-    else
-        log("Hmm, stray %s: '%s'", __func__, pMessage.c_str());
+    s_pObs->Status(msg, peer, key);
 }
diff --git a/lib/Utils/CommLayerInner.h b/lib/Utils/CommLayerInner.h
index d161cfc7..9c22968a 100644
--- a/lib/Utils/CommLayerInner.h
+++ b/lib/Utils/CommLayerInner.h
@@ -9,15 +9,19 @@ void init_daemon_logging(CObserver *pObs);
  * Set client's name (dbus ID). NULL unsets it.
  */
 void set_client_name(const char* name);
-/* Ask a client to warn the user about a non-fatal, but unexpected condition.
+
+/*
+ * Ask a client to warn the user about a non-fatal, but unexpected condition.
  * In GUI, it will usually be presented as a popup message.
+ * Usually there is no need to call it directly, just use [p]error_msg().
  */
-void warn_client(const std::string& pMessage);
-/* Logs a message to a client.
+//now static: void warn_client(const char *msg);
+
+/*
+ * Logs a message to a client.
  * In UI, it will usually appear as a new status line message in GUI,
  * or as a new message line in CLI.
  */
-void update_client(const std::string& pMessage);
-
-#endif /* COMMLAYERINNER_H_ */
+void update_client(const char *fmt, ...);
 
+#endif
diff --git a/lib/Utils/logging.cpp b/lib/Utils/logging.cpp
index f70d23f3..cae609bc 100644
--- a/lib/Utils/logging.cpp
+++ b/lib/Utils/logging.cpp
@@ -7,19 +7,18 @@
 #include <syslog.h>
 
 int xfunc_error_retval = EXIT_FAILURE;
-
 int g_verbose;
+int logmode = LOGMODE_STDIO;
+const char *msg_prefix = "";
+const char *msg_eol = "\n";
+void (*g_custom_logger)(const char*);
 
 void xfunc_die(void)
 {
 	exit(xfunc_error_retval);
 }
 
-const char *msg_prefix = "";
-const char *msg_eol = "\n";
-int logmode = LOGMODE_STDIO;
-
-void verror_msg(const char *s, va_list p, const char* strerr)
+static void verror_msg_helper(const char *s, va_list p, const char* strerr, int flags)
 {
 	char *msg;
 	int prefix_len, strerr_len, msgeol_len, used;
@@ -27,9 +26,6 @@ void verror_msg(const char *s, va_list p, const char* strerr)
 	if (!logmode)
 		return;
 
-	if (!s) /* nomsg[_and_die] uses NULL fmt */
-		s = ""; /* some libc don't like printf(NULL) */
-
 	used = vasprintf(&msg, s, p);
 	if (used < 0)
 		return;
@@ -51,7 +47,7 @@ void verror_msg(const char *s, va_list p, const char* strerr)
 		memcpy(msg, msg_prefix, prefix_len);
 	}
 	if (strerr) {
-		if (s[0]) { /* not perror_nomsg? */
+		if (s[0]) {
 			msg[used++] = ':';
 			msg[used++] = ' ';
 		}
@@ -60,24 +56,26 @@ void verror_msg(const char *s, va_list p, const char* strerr)
 	}
 	strcpy(&msg[used], msg_eol);
 
-	if (logmode & LOGMODE_STDIO) {
+	if (flags & LOGMODE_STDIO) {
 		fflush(stdout);
 		full_write(STDERR_FILENO, msg, used + msgeol_len);
 	}
-	if (logmode & LOGMODE_SYSLOG) {
+	if (flags & LOGMODE_SYSLOG) {
 		syslog(LOG_ERR, "%s", msg + prefix_len);
 	}
+	if ((flags & LOGMODE_CUSTOM) && g_custom_logger) {
+		g_custom_logger(msg + prefix_len);
+	}
 	free(msg);
 }
 
-void error_msg_and_die(const char *s, ...)
+void log_msg(const char *s, ...)
 {
 	va_list p;
 
 	va_start(p, s);
-	verror_msg(s, p, NULL);
+	verror_msg_helper(s, p, NULL, logmode);
 	va_end(p);
-	xfunc_die();
 }
 
 void error_msg(const char *s, ...)
@@ -85,8 +83,18 @@ void error_msg(const char *s, ...)
 	va_list p;
 
 	va_start(p, s);
-	verror_msg(s, p, NULL);
+	verror_msg_helper(s, p, NULL, (logmode | LOGMODE_CUSTOM));
+	va_end(p);
+}
+
+void error_msg_and_die(const char *s, ...)
+{
+	va_list p;
+
+	va_start(p, s);
+	verror_msg_helper(s, p, NULL, (logmode | LOGMODE_CUSTOM));
 	va_end(p);
+	xfunc_die();
 }
 
 void perror_msg_and_die(const char *s, ...)
@@ -95,7 +103,7 @@ void perror_msg_and_die(const char *s, ...)
 
 	va_start(p, s);
 	/* Guard against "<error message>: Success" */
-	verror_msg(s, p, errno ? strerror(errno) : NULL);
+	verror_msg_helper(s, p, errno ? strerror(errno) : NULL, (logmode | LOGMODE_CUSTOM));
 	va_end(p);
 	xfunc_die();
 }
@@ -106,7 +114,7 @@ void perror_msg(const char *s, ...)
 
 	va_start(p, s);
 	/* Guard against "<error message>: Success" */
-	verror_msg(s, p, errno ? strerror(errno) : NULL);
+	verror_msg_helper(s, p, errno ? strerror(errno) : NULL, (logmode | LOGMODE_CUSTOM));
 	va_end(p);
 }
 
@@ -122,5 +130,5 @@ void simple_perror_msg(const char *s)
 
 void die_out_of_memory(void)
 {
-    error_msg_and_die("Out of memory, exiting");
+	error_msg_and_die("Out of memory, exiting");
 }