[Spice-devel] [PATCH spice-xpi 4/5] Switch from log4cpp to glib
Marc-André Lureau
marcandre.lureau at gmail.com
Fri Feb 24 06:20:24 PST 2012
GLib is used in the defacto library for desktop projects. It includes
logging facilities that can be filtered by domain and level, and can
be redirected or formated in various ways. Instead of pulling an extra
large depedency for a small plugin, let's use what is common to the
desktop.
Afaict, nobody else is using log4cpp in Fedora. The configuration file
is not user friendly. In comparison, you can just run
"G_MESSAGES_DEBUG=SpiceXPI firefox" and easily modify logging via
environment variable. There is no logging to file, and there is no
configuration file. If you want to redirect to file, you can do so via
redirection, or other mechanisms (ldpreload, environment..).
---
SpiceXPI/Makefile.am | 7 +-
SpiceXPI/logger.ini | 16 ----
SpiceXPI/src/plugin/Makefile.am | 33 ++++-----
SpiceXPI/src/plugin/controller.cpp | 21 ++----
SpiceXPI/src/plugin/debug.h | 143 ------------------------------------
SpiceXPI/src/plugin/plugin.cpp | 65 ++++++-----------
configure.ac | 8 +-
7 files changed, 51 insertions(+), 242 deletions(-)
delete mode 100644 SpiceXPI/logger.ini
delete mode 100644 SpiceXPI/src/plugin/debug.h
diff --git a/SpiceXPI/Makefile.am b/SpiceXPI/Makefile.am
index a2dee55..d4bf15a 100644
--- a/SpiceXPI/Makefile.am
+++ b/SpiceXPI/Makefile.am
@@ -1,10 +1,8 @@
-SUBDIRS=src
+SUBDIRS = src
-EXTRA_DIST = logger.ini
spicedatadir = $(datadir)/spice
-spicedata_DATA = logger.ini
-DISTDIR=dist
+DISTDIR = dist
all-local: SpiceXPI.xpi
@@ -16,7 +14,6 @@ SpiceXPI.xpi: $(srcdir)/src/install.rdf src/plugin/nsISpicec.xpt src/plugin/.lib
@cp $(srcdir)/src/install.rdf $(DISTDIR)
@cp src/plugin/*.xpt $(DISTDIR)/plugins
@cp src/plugin/.libs/libnsISpicec.so $(DISTDIR)/plugins/nsISpicec.so
- @cp $(srcdir)/logger.ini $(DISTDIR)/plugins/logger.ini
@(cd $(DISTDIR); zip -q -r ../$@ .)
distclean-local:
diff --git a/SpiceXPI/logger.ini b/SpiceXPI/logger.ini
deleted file mode 100644
index 272b60d..0000000
--- a/SpiceXPI/logger.ini
+++ /dev/null
@@ -1,16 +0,0 @@
-#
-# logger.ini - Spice Client Logger Configuration File
-#
-
-# Set root logger level
-# Optional levels: EMERG, FATAL, ALERT, CRIT, ERROR, WARN, NOTICE, INFO, DEBUG, NOTSET
-
-log4j.rootCategory=INFO, R
-
-log4j.appender.R=org.apache.log4j.RollingFileAppender
-log4j.appender.R.fileName=${HOME}/.spicec/spice-xpi.log
-log4j.appender.R.maxFileSize=1000000
-log4j.appender.R.maxBackupIndex=1
-log4j.appender.R.append=true
-log4j.appender.R.layout=org.apache.log4j.PatternLayout
-log4j.appender.R.layout.ConversionPattern=%d %-5p %m%n
diff --git a/SpiceXPI/src/plugin/Makefile.am b/SpiceXPI/src/plugin/Makefile.am
index e409d94..08d89d7 100644
--- a/SpiceXPI/src/plugin/Makefile.am
+++ b/SpiceXPI/src/plugin/Makefile.am
@@ -4,33 +4,31 @@ FIREFOX_APPID = {ec8030f7-c20a-464f-9b0e-13a3a9e97384}
extensiondir = $(libdir)/mozilla
SDK_INCLUDE_DIR = `pkg-config --variable=idldir libxul`
-INCLUDES = \
+plugindir=$(extensiondir)/plugins
+
+plugin_LTLIBRARIES = libnsISpicec.la
+
+libnsISpicec_la_LDFLAGS = -avoid-version -module
+libnsISpicec_la_CPPFLAGS = \
+ -I$(XUL_INCLUDEDIR) \
+ -I$(XUL_INCLUDEDIR)/dom \
+ -I$(XUL_INCLUDEDIR)/necko \
+ -I$(XUL_INCLUDEDIR)/plugin \
+ -I$(XUL_INCLUDEDIR)/string \
-I$(top_srcdir)/common \
- $(XUL_CFLAGS) \
+ $(GLIB_CFLAGS) \
$(SPICE_PROTOCOL_CFLAGS) \
- $(LOG4CPP_CFLAGS) \
+ $(XUL_CFLAGS) \
-DCAIRO_CANVAS_ACCESS_TEST \
-DCAIRO_CANVAS_CACHE \
-DCAIRO_CANVAS_NO_CHUNKS \
+ -DG_LOG_DOMAIN=\"SpiceXPI\" \
-DMOZILLA_INTERNAL_API \
-DXP_UNIX \
$(NULL)
-plugindir=$(extensiondir)/plugins
-
-plugin_LTLIBRARIES = libnsISpicec.la
-
-libnsISpicec_la_LDFLAGS = -avoid-version -module
-libnsISpicec_la_CPPFLAGS = \
- -I $(XUL_INCLUDEDIR) \
- -I $(XUL_INCLUDEDIR)/dom \
- -I $(XUL_INCLUDEDIR)/necko \
- -I $(XUL_INCLUDEDIR)/plugin \
- -I $(XUL_INCLUDEDIR)/string \
- $(NULL)
-
libnsISpicec_la_LIBADD = \
- $(LOG4CPP_LIBS) \
+ $(GLIB_LIBS) \
$(XUL_LIBS) \
$(NULL)
@@ -39,7 +37,6 @@ libnsISpicec_la_SOURCES = \
$(top_srcdir)/common/rederrorcodes.h \
controller.cpp \
controller.h \
- debug.h \
np_entry.cpp \
npn_gate.cpp \
npp_gate.cpp \
diff --git a/SpiceXPI/src/plugin/controller.cpp b/SpiceXPI/src/plugin/controller.cpp
index 256dbf9..131cfc5 100644
--- a/SpiceXPI/src/plugin/controller.cpp
+++ b/SpiceXPI/src/plugin/controller.cpp
@@ -44,6 +44,7 @@
#include <cstdlib>
#include <cstring>
#include <cerrno>
+#include <glib.h>
extern "C" {
# include <stdint.h>
@@ -53,17 +54,11 @@ extern "C" {
# include <sys/un.h>
}
-#include "debug.h"
#include "rederrorcodes.h"
#include "controller.h"
const char *channel_names[] = { "dummy", "main", "display", "inputs", "cursor", "playback", "record" };
-void QErrorHandler(int err, const char *custom_string)
-{
- LOG_DEBUG("Something went wrong: " << custom_string << ", " << err);
-}
-
SpiceController::SpiceController():
m_client_socket(-1)
{
@@ -77,7 +72,7 @@ SpiceController::SpiceController(const std::string &name):
SpiceController::~SpiceController()
{
- LOG_TRACE("");
+ g_debug(G_STRFUNC);
Disconnect();
}
@@ -96,7 +91,7 @@ int SpiceController::Connect()
{
if ((m_client_socket = socket(AF_UNIX, SOCK_STREAM, 0)) == -1)
{
- QErrorHandler(errno, "SpiceController::Connect socket create error");
+ g_critical("controller socket: %s", g_strerror(errno));
return -1;
}
}
@@ -108,12 +103,11 @@ int SpiceController::Connect()
int rc = connect(m_client_socket, (struct sockaddr *) &remote, strlen(remote.sun_path) + sizeof(remote.sun_family));
if (rc == -1)
{
- QErrorHandler(errno, "connect error");
- LOG_DEBUG("Connect Error");
+ g_critical("controller connect: %s", g_strerror(errno));
}
else
{
- LOG_DEBUG("Connected!");
+ g_debug("controller connected");
}
return rc;
@@ -152,9 +146,8 @@ uint32_t SpiceController::Write(const void *lpBuffer, uint32_t nBytesToWrite)
if (len != (ssize_t)nBytesToWrite)
{
- LOG_WARN("send error, bytes to write = " << nBytesToWrite <<
- ", bytes actually written = " << len << ", errno = " << errno);
- QErrorHandler(1, "send error");
+ g_warning("incomplete send, bytes to write = %lu, bytes written = %d: %s",
+ nBytesToWrite, len, g_strerror(errno));
}
return len;
diff --git a/SpiceXPI/src/plugin/debug.h b/SpiceXPI/src/plugin/debug.h
deleted file mode 100644
index 4b58491..0000000
--- a/SpiceXPI/src/plugin/debug.h
+++ /dev/null
@@ -1,143 +0,0 @@
-/* ***** BEGIN LICENSE BLOCK *****
- * Version: MPL 1.1/GPL 2.0/LGPL 2.1
- *
- * The contents of this file are subject to the Mozilla Public License Version
- * 1.1 (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- * http://www.mozilla.org/MPL/
- *
- * Software distributed under the License is distributed on an "AS IS" basis,
- * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
- * for the specific language governing rights and limitations under the
- * License.
- *
- * Copyright 2009-2011, Red Hat Inc.
- * Based on mozilla.org's scriptable plugin example
- *
- * The Original Code is mozilla.org code.
- *
- * The Initial Developer of the Original Code is
- * Netscape Communications Corporation.
- * Portions created by the Initial Developer are Copyright (C) 1998
- * the Initial Developer. All Rights Reserved.
- *
- * Contributor(s):
- * Uri Lublin
- *
- * Alternatively, the contents of this file may be used under the terms of
- * either the GNU General Public License Version 2 or later (the "GPL"), or
- * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
- * in which case the provisions of the GPL or the LGPL are applicable instead
- * of those above. If you wish to allow use of your version of this file only
- * under the terms of either the GPL or the LGPL, and not to allow others to
- * use your version of this file under the terms of the MPL, indicate your
- * decision by deleting the provisions above and replace them with the notice
- * and other provisions required by the GPL or the LGPL. If you do not delete
- * the provisions above, a recipient may use your version of this file under
- * the terms of any one of the MPL, the GPL or the LGPL.
- *
- * ***** END LICENSE BLOCK ***** */
-
-#ifndef DEBUG_H
-#define DEBUG_H
-
-#include <sstream>
-
-#include <log4cpp/Category.hh>
-#include <log4cpp/convenience.h>
-
-#define ON_PANIC() ::abort()
-
-#ifdef RED_DEBUG
-
-# ifdef WIN32
-# define ASSERTBREAK DebugBreak()
-# else
-# define ASSERTBREAK ::abort()
-# endif
-
-# define ASSERT(x) if (!(x)) { \
- printf("%s: ASSERT %s failed\n", __FUNCTION__, #x); \
- ASSERTBREAK; \
-}
-
-#else
-# define ASSERT(cond)
-#endif
-
-#ifdef __GNUC__
-static inline std::string pretty_func_to_func_name(const std::string& f_name)
-{
- std::string name(f_name);
- std::string::size_type end_pos = f_name.find('(');
- if (end_pos == std::string::npos) {
- return f_name;
- }
- std::string::size_type start = f_name.rfind(' ', end_pos);
- if (start == std::string::npos) {
- return f_name;
- }
- end_pos -= ++start;
- return name.substr(start, end_pos);
-}
-# define FUNC_NAME pretty_func_to_func_name(__PRETTY_FUNCTION__).c_str()
-#else
-# define FUNC_NAME __FUNCTION__
-#endif
-
-LOG4CPP_LOGGER("spice")
-
-#define LOG(func, message) { \
- std::ostringstream os; \
- os << FUNC_NAME; \
- os << ": "; \
- os << message; \
- func(logger, os.str().c_str()); \
-}
-
-// Used to log just about everything that might be useful (or not).
-#define LOG_TRACE(message) LOG(LOG4CPP_DEBUG, message)
-
-// Used to log debug information which doesn't inflate the log with useless
-// and repeated messages.
-#define LOG_DEBUG(message) LOG(LOG4CPP_DEBUG, message)
-
-// Used to log the program flow.
-#define LOG_INFO(message) LOG(LOG4CPP_INFO, message)
-
-// Used to log errors which are handled by the program.
-#define LOG_WARN(message) LOG(LOG4CPP_WARN, message)
-
-// Used to log errors that can't be handled by the program.
-#define LOG_ERROR(message) LOG(LOG4CPP_ERROR, message)
-
-// Used to log errors that can't happen (even if they do!).
-#define LOG_FATAL(message) { \
- LOG(LOG4CPP_EMERG, message); \
- ON_PANIC(); \
-}
-
-#if 0
-# define DBGLEVEL 1000
-
-# define DBG(level, msg) { \
- if (level <= DBGLEVEL) { \
- std::ostringstream os; \
- os << __FUNCTION__; \
- os << ": "; \
- os << msg; \
- os << "\n"; \
- std::cout << os.str(); \
- std::cout.flush(); \
- } \
-}
-#endif
-
-#define PANIC(msg) LOG_FATAL(msg)
-#define WARN(msg) LOG_WARN(msg)
-#define ERR(msg) LOG_ERROR(msg)
-#define NOTIFY(msg) LOG_INFO(msg)
-#define INFO(msg) LOG_INFO(msg)
-#define DBG(level, msg) LOG_DEBUG(msg)
-
-#endif // DEBUG_H
diff --git a/SpiceXPI/src/plugin/plugin.cpp b/SpiceXPI/src/plugin/plugin.cpp
index c3d2d91..2860d5e 100644
--- a/SpiceXPI/src/plugin/plugin.cpp
+++ b/SpiceXPI/src/plugin/plugin.cpp
@@ -46,6 +46,7 @@
#include <sys/types.h>
#include <sys/wait.h>
+#include <sys/stat.h>
#include <stdlib.h>
#include <dlfcn.h>
@@ -54,6 +55,7 @@
#include <string>
#include <sstream>
#include <signal.h>
+#include <glib.h>
#include "nsCOMPtr.h"
@@ -73,15 +75,11 @@ static NS_DEFINE_CID(kPluginManagerCID, NS_PLUGINMANAGER_CID);
#include <nsIServiceManager.h>
#include <nsISupportsUtils.h> // some usefule macros are defined here
-
-#include <log4cpp/PropertyConfigurator.hh>
-#include <log4cpp/BasicConfigurator.hh>
-
#include <fstream>
#include <set>
+
#include "config.h"
#include "controller.h"
-#include "debug.h"
#include "plugin.h"
#include "nsScriptablePeer.h"
@@ -93,7 +91,6 @@ namespace {
const std::string PLUGIN_NAME = "Spice Firefox Plugin " + ver;
const std::string MIME_TYPES_DESCRIPTION = "application/x-spice:qsc:" + PLUGIN_NAME;
const std::string PLUGIN_DESCRIPTION = PLUGIN_NAME + " Spice Client wrapper for firefox";
- const std::string LOGGER_CONFIG = "/usr/share/spice/logger.ini";
// helper function for string copy
char *stringCopy(const std::string &src)
@@ -197,18 +194,6 @@ nsPluginInstance::nsPluginInstance(NPP aInstance):
char tmp_dir[] = "/tmp/spicec-XXXXXX";
m_tmp_dir = mkdtemp(tmp_dir);
- // configure log4cpp
- std::ifstream log_init(LOGGER_CONFIG.c_str());
- if (log_init.good())
- {
- log_init.close();
- log4cpp::PropertyConfigurator::configure(LOGGER_CONFIG.c_str());
- }
- else
- {
- log4cpp::BasicConfigurator::configure();
- }
-
m_connected_status = -2;
struct sigaction chld;
@@ -565,7 +550,7 @@ void nsPluginInstance::Connect()
socket_file += "/spice-xpi";
if (setenv("SPICE_XPI_SOCKET", socket_file.c_str(), 1))
{
- LOG_ERROR ("could not set SPICE_XPI_SOCKET env variable");
+ g_critical("could not set SPICE_XPI_SOCKET env variable");
return;
}
@@ -577,27 +562,27 @@ void nsPluginInstance::Connect()
}
pid_t child = fork();
- LOG_DEBUG("child pid: " << child);
+ g_debug("child pid: %llu", (guint64)child);
if (child == 0)
{
- close (pipe_fds[1]);
+ close(pipe_fds[1]);
pipe_fds[1] = -1;
char c;
if (read(pipe_fds[0], &c, 1) != 0)
- LOG_ERROR("Invalid read on pipe");
+ g_critical("Error while reading on pipe: %s", g_strerror(errno));
- close (pipe_fds[0]);
+ close(pipe_fds[0]);
pipe_fds[0] = -1;
execl("/usr/libexec/spice-xpi-client", "/usr/libexec/spice-xpi-client", NULL);
- LOG_ERROR("ERROR failed to run spice-xpi-client");
+ g_message("failed to run spice-xpi-client, running spicec instead");
// TODO: temporary fallback for backward compatibility
execl("/usr/bin/spicec", "/usr/bin/spicec", "--controller", NULL);
- LOG_ERROR("ERROR failed to run spicec fallback");
+ g_critical("ERROR failed to run spicec fallback");
- exit(1);
+ exit(EXIT_FAILURE);
}
else
{
@@ -613,7 +598,7 @@ void nsPluginInstance::Connect()
if (m_external_controller.Connect(10) != 0)
{
- LOG_ERROR ("could not connect to spice client controller");
+ g_critical("could not connect to spice client controller");
return;
}
@@ -636,7 +621,7 @@ void nsPluginInstance::Connect()
}
else
{
- LOG_ERROR ("could not open truststore temp file");
+ g_critical("could not open truststore temp file");
close(fd);
unlink(m_trust_store_file.c_str());
m_trust_store_file.clear();
@@ -645,7 +630,7 @@ void nsPluginInstance::Connect()
}
else
{
- LOG_ERROR ("could not create truststore temp file. errno=" << errno);
+ g_critical("could not create truststore temp file: %s", g_strerror(errno));
return;
}
@@ -689,7 +674,7 @@ void nsPluginInstance::Connect()
void nsPluginInstance::Show()
{
- LOG_DEBUG("sending show message");
+ g_debug("sending show message");
SendMsg(CONTROLLER_SHOW);
}
@@ -733,7 +718,7 @@ void nsPluginInstance::CallOnDisconnected(int code)
NPObject *window = NULL;
if (NPN_GetValue(m_instance, NPNVWindowNPObject, &window) != NPERR_NO_ERROR)
{
- LOG_ERROR("could not get browser window, when trying to call OnDisconnected");
+ g_critical("could not get browser window, when trying to call OnDisconnected");
return;
}
@@ -741,20 +726,20 @@ void nsPluginInstance::CallOnDisconnected(int code)
NPIdentifier id_on_disconnected = NPN_GetStringIdentifier("OnDisconnected");
if (!id_on_disconnected)
{
- LOG_ERROR("could not find OnDisconnected identifier");
+ g_critical("could not find OnDisconnected identifier");
return;
}
NPVariant var_on_disconnected;
if (!NPN_GetProperty(m_instance, window, id_on_disconnected, &var_on_disconnected))
{
- LOG_ERROR("could not get OnDisconnected function");
+ g_critical("could not get OnDisconnected function");
return;
}
if (!NPVARIANT_IS_OBJECT(var_on_disconnected))
{
- LOG_ERROR("OnDisconnected is not object");
+ g_critical("OnDisconnected is not object");
return;
}
@@ -767,13 +752,9 @@ void nsPluginInstance::CallOnDisconnected(int code)
NPVariant args[] = { arg };
if (NPN_InvokeDefault(m_instance, call_on_disconnected, args, sizeof(args) / sizeof(args[0]), &void_result))
- {
- LOG_DEBUG("OnDisconnected successfuly called");
- }
+ g_debug("OnDisconnected successfuly called");
else
- {
- LOG_ERROR("could not call OnDisconnected");
- }
+ g_critical("could not call OnDisconnected");
// cleanup
NPN_ReleaseObject(window);
@@ -782,14 +763,14 @@ void nsPluginInstance::CallOnDisconnected(int code)
void nsPluginInstance::SigchldRoutine(int sig, siginfo_t *info, void *uap)
{
- LOG_DEBUG("child finished, pid: " << info->si_pid);
+ g_debug("child finished, pid: %llu", (guint64)info->si_pid);
int exit_code;
waitpid(info->si_pid, &exit_code, 0);
if (!getenv("SPICE_XPI_DEBUG")) {
nsPluginInstance *fake_this = s_children[info->si_pid];
if (fake_this == NULL) {
- LOG_ERROR("Invalid children signal");
+ g_critical("Invalid children signal");
return;
}
diff --git a/configure.ac b/configure.ac
index cb16269..4614083 100644
--- a/configure.ac
+++ b/configure.ac
@@ -18,14 +18,14 @@ AM_PROG_CC_C_O
dnl =========================================================================
dnl Check deps
-PKG_CHECK_MODULES(LOG4CPP, log4cpp)
-AC_SUBST(LOG4CPP_CFLAGS)
-AC_SUBST(LOG4CPP_LIBS)
-
AC_CONFIG_SUBDIRS([spice-protocol])
SPICE_PROTOCOL_CFLAGS='-I ${top_srcdir}/spice-protocol'
AC_SUBST(SPICE_PROTOCOL_CFLAGS)
+PKG_CHECK_MODULES(GLIB, glib-2.0)
+AC_SUBST(GLIB_CFLAGS)
+AC_SUBST(GLIB_LIBS)
+
# The explicit nspr dep is needed because libxul-embedding
# in RHEL5 is missing the Requires
PKG_CHECK_MODULES(XUL, libxul-embedding >= 1.9 nspr >= 4.7.1)
--
1.7.7.6
More information about the Spice-devel
mailing list