Remove logmask stuff from backend; switch logging callback to new pm_printf

Remove the logmask functionality from the backend as it has been moved to
the frontend, and change the logging callback function to use pm_printf.

In addition, make much better use of va_list- use the args list instead
of a arbitrarily chosen string to print to in the logging functions.

Signed-off-by: Dan McGee <dan@archlinux.org>

lib/libalpm/alpm.h

@@ -29,6 +29,7 @@ extern "C" {
 #endif
 
 #include <time.h> /* for time_t */
+#include <stdarg.h> /* for va_list */
 
 /*
  * Arch Linux Package Management library
@@ -70,7 +71,7 @@ typedef enum _pmloglevel_t {
 	PM_LOG_FUNCTION = 0x10
 } pmloglevel_t;
 
-typedef void (*alpm_cb_log)(pmloglevel_t, char *);
+typedef void (*alpm_cb_log)(pmloglevel_t, char *, va_list);
 int alpm_logaction(char *fmt, ...);
 
 /*
@@ -91,9 +92,6 @@ void alpm_option_set_logcb(alpm_cb_log cb);
 alpm_cb_download alpm_option_get_dlcb();
 void alpm_option_set_dlcb(alpm_cb_download cb);
 
-unsigned short alpm_option_get_logmask();
-void alpm_option_set_logmask(unsigned short mask);
-
 const char *alpm_option_get_root();
 void alpm_option_set_root(const char *root);
 

lib/libalpm/handle.c

@@ -78,7 +78,6 @@ pmhandle_t *_alpm_handle_new()
 //#else
 	handle->access = PM_ACCESS_RW;
 #endif
-	handle->logmask = PM_LOG_ERROR | PM_LOG_WARNING;
 	handle->root = NULL;
 	handle->dbpath = NULL;
 	handle->cachedir = NULL;
@@ -124,7 +123,6 @@ void _alpm_handle_free(pmhandle_t *handle)
 
 alpm_cb_log SYMEXPORT alpm_option_get_logcb() { return (handle ? handle->logcb : NULL); }
 alpm_cb_download SYMEXPORT alpm_option_get_dlcb() { return (handle ? handle->dlcb : NULL); }
-unsigned short SYMEXPORT alpm_option_get_logmask() { return handle->logmask; }
 const char SYMEXPORT *alpm_option_get_root() { return handle->root; }
 const char SYMEXPORT *alpm_option_get_dbpath() { return handle->dbpath; }
 const char SYMEXPORT *alpm_option_get_cachedir() { return handle->cachedir; }
@@ -149,10 +147,10 @@ void SYMEXPORT alpm_option_set_logcb(alpm_cb_log cb) { handle->logcb = cb; }
 
 void SYMEXPORT alpm_option_set_dlcb(alpm_cb_download cb) { handle->dlcb = cb; }
 
-void SYMEXPORT alpm_option_set_logmask(unsigned short mask) { handle->logmask = mask; }
-
 void SYMEXPORT alpm_option_set_root(const char *root)
 {
+	ALPM_LOG_FUNC;
+
 	if(handle->root) FREE(handle->root);
 	/* According to the man page, realpath is safe to use IFF the second arg is
 	 * NULL. */
@@ -172,8 +170,6 @@ void SYMEXPORT alpm_option_set_root(const char *root)
 		handle->root = calloc(rootlen+1, sizeof(char));
 		strncpy(handle->root, realroot, rootlen);
 		handle->root[rootlen-1] = '/';
-		_alpm_log(PM_LOG_DEBUG, _("option 'root' = %s"), handle->root);
-
 	}
 	if(realroot) {
 		free(realroot);
@@ -182,6 +178,8 @@ void SYMEXPORT alpm_option_set_root(const char *root)
 
 void SYMEXPORT alpm_option_set_dbpath(const char *dbpath)
 {
+	ALPM_LOG_FUNC;
+
 	if(handle->dbpath) FREE(handle->dbpath);
 	if(dbpath) {
 		/* verify dbpath ends in a '/' */
@@ -192,12 +190,13 @@ void SYMEXPORT alpm_option_set_dbpath(const char *dbpath)
 		handle->dbpath = calloc(dbpathlen+1, sizeof(char));
 		strncpy(handle->dbpath, dbpath, dbpathlen);
 		handle->dbpath[dbpathlen-1] = '/';
-		_alpm_log(PM_LOG_DEBUG, _("option 'dbpath' = %s"), handle->dbpath);
 	}
 }
 
 void SYMEXPORT alpm_option_set_cachedir(const char *cachedir)
 {
+	ALPM_LOG_FUNC;
+
 	if(handle->cachedir) FREE(handle->cachedir);
 	if(cachedir) {
 		/* verify cachedir ends in a '/' */
@@ -208,7 +207,6 @@ void SYMEXPORT alpm_option_set_cachedir(const char *cachedir)
 		handle->cachedir = calloc(cachedirlen+1, sizeof(char));
 		strncpy(handle->cachedir, cachedir, cachedirlen);
 		handle->cachedir[cachedirlen-1] = '/';
-		_alpm_log(PM_LOG_DEBUG, _("option 'cachedir' = %s"), handle->cachedir);
 	}
 }
 
@@ -231,6 +229,8 @@ void SYMEXPORT alpm_option_set_logfile(const char *logfile)
 
 void SYMEXPORT alpm_option_set_lockfile(const char *lockfile)
 {
+	ALPM_LOG_FUNC;
+
 	if(handle->lockfile) FREE(handle->lockfile);
 	if(lockfile) {
 		handle->lockfile = strdup(lockfile);

lib/libalpm/handle.h

@@ -48,7 +48,6 @@ typedef struct _pmhandle_t {
 	/* options */
   alpm_cb_log logcb;				/* Log callback function */
 	alpm_cb_download dlcb;    /* Download callback function */
-  unsigned short logmask;		/* Output mask for logging functions */ /* TODO move to frontend */
 	char *root;								/* Root path, default '/' */
 	char *dbpath;							/* Base path to pacman's DBs */
 	char *cachedir;						/* Base path to pacman's cache */

lib/libalpm/log.c

@@ -43,7 +43,7 @@
  */
 int SYMEXPORT alpm_logaction(char *fmt, ...)
 {
-	char str[LOG_STR_LEN];
+	int ret;
 	va_list args;
 
 	ALPM_LOG_FUNC;
@@ -52,7 +52,7 @@ int SYMEXPORT alpm_logaction(char *fmt, ...)
 	ASSERT(handle != NULL, RET_ERR(PM_ERR_HANDLE_NULL, -1));
 
 	va_start(args, fmt);
-	vsnprintf(str, LOG_STR_LEN, fmt, args);
+	ret = _alpm_logaction(handle->usesyslog, handle->logfd, fmt, args);
 	va_end(args);
 
 	/* TODO	We should add a prefix to log strings depending on who called us.
@@ -66,28 +66,23 @@ int SYMEXPORT alpm_logaction(char *fmt, ...)
 	 *   kpacman: "KPACMAN"
 	 * This would allow us to share the log file between several frontends
 	 * and know who does what */
-	return(_alpm_logaction(handle->usesyslog, handle->logfd, str));
+	return(ret);
 }
 
 /** @} */
 
 void _alpm_log(pmloglevel_t flag, char *fmt, ...)
 {
+	va_list args;
 	alpm_cb_log logcb = alpm_option_get_logcb();
+
 	if(logcb == NULL) {
 		return;
 	}
 
-	if(flag & alpm_option_get_logmask()) {
-		char str[LOG_STR_LEN];
-		va_list args;
-
-		va_start(args, fmt);
-		vsnprintf(str, LOG_STR_LEN, fmt, args);
-		va_end(args);
-
-		logcb(flag, str);
-	}
+	va_start(args, fmt);
+	logcb(flag, fmt, args);
+	va_end(args);
 }
 
 /* vim: set ts=2 sw=2 noet: */

lib/libalpm/log.h

@@ -23,8 +23,6 @@
 
 #include "alpm.h"
 
-#define LOG_STR_LEN 1024
-
 #ifdef PACMAN_DEBUG
 /* Log funtion entry points if debugging is enabled */
 #define ALPM_LOG_FUNC _alpm_log(PM_LOG_FUNCTION, "Enter %s", __func__)

lib/libalpm/server.c

@@ -249,15 +249,7 @@ int _alpm_downloadfiles_forreal(alpm_list_t *servers, const char *localpath,
 				/* 10s timeout - TODO make a config option */
 				downloadTimeout = 10000;
 
-			 	/* Make libdownload super verbose... worthwhile for testing */
-				if(alpm_option_get_logmask() & PM_LOG_DOWNLOAD) {
-						downloadDebug = 1;
-				}
-				if(alpm_option_get_logmask() & PM_LOG_DEBUG) {
-						dlf = downloadXGet(fileurl, &ust, (handle->nopassiveftp ? "v" : "vp"));
-				} else {
-						dlf = downloadXGet(fileurl, &ust, (handle->nopassiveftp ? "" : "p"));
-				}
+				dlf = downloadXGet(fileurl, &ust, (handle->nopassiveftp ? "" : "p"));
 
 				if(downloadLastErrCode != 0 || dlf == NULL) {
 					_alpm_log(PM_LOG_ERROR, _("failed retrieving file '%s' from %s : %s"),

lib/libalpm/util.c

@@ -379,12 +379,12 @@ int _alpm_rmrf(const char *path)
 	return(0);
 }
 
-int _alpm_logaction(unsigned short usesyslog, FILE *f, const char *str)
+int _alpm_logaction(unsigned short usesyslog, FILE *f, const char *fmt, va_list args)
 {
-	_alpm_log(PM_LOG_DEBUG, _("logaction called: %s"), str);
+	int ret = 0;
 
 	if(usesyslog) {
-		syslog(LOG_WARNING, "%s", str);
+		vsyslog(LOG_WARNING, fmt, args);
 	}
 
 	if(f) {
@@ -395,14 +395,15 @@ int _alpm_logaction(unsigned short usesyslog, FILE *f, const char *str)
 		tm = localtime(&t);
 
 		/* Use ISO-8601 date format */
-		fprintf(f, "[%04d-%02d-%02d %02d:%02d] %s\n",
+		fprintf(f, "[%04d-%02d-%02d %02d:%02d] ",
 						tm->tm_year+1900, tm->tm_mon+1, tm->tm_mday,
-						tm->tm_hour, tm->tm_min, str);
-
+						tm->tm_hour, tm->tm_min);
+		ret = vfprintf(f, fmt, args);
+		fprintf(f, "\n");
 		fflush(f);
 	}
 
-	return(0);
+	return(ret);
 }
 
 int _alpm_ldconfig(const char *root)

lib/libalpm/util.h

@@ -53,7 +53,7 @@ int _alpm_lckmk();
 int _alpm_lckrm();
 int _alpm_unpack(const char *archive, const char *prefix, const char *fn);
 int _alpm_rmrf(const char *path);
-int _alpm_logaction(unsigned short usesyslog, FILE *f, const char *str);
+int _alpm_logaction(unsigned short usesyslog, FILE *f, const char *fmt, va_list args);
 int _alpm_ldconfig(const char *root);
 void _alpm_time2string(time_t t, char *buffer);
 int _alpm_str_cmp(const void *s1, const void *s2);

src/pacman/callback.c

@@ -593,54 +593,13 @@ void cb_dl_progress(const char *filename, int xfered, int total)
 }
 
 /* Callback to handle notifications from the library */
-void cb_log(pmloglevel_t level, char *msg)
+void cb_log(pmloglevel_t level, char *fmt, va_list args)
 {
-	char str[LOG_STR_LEN] = "";
-
-	if(!strlen(msg)) {
+	if(!strlen(fmt)) {
 		return;
 	}
 
-	switch(level) {
-		case PM_LOG_DEBUG:
-			sprintf(str, _("debug"));
-		break;
-		case PM_LOG_ERROR:
-			sprintf(str, _("error"));
-		break;
-		case PM_LOG_WARNING:
-			sprintf(str, _("warning"));
-		break;
-		case PM_LOG_FUNCTION:
-		  /* TODO we should increase the indent level when this occurs so we can see
-			 * program flow easier.  It'll be fun
-			 */
-			sprintf(str, _("function"));
-		break;
-		default:
-			sprintf(str, "???");
-		break;
-	}
-
-#ifdef PACMAN_DEBUG
-	/* If debug is on, we'll timestamp the output */
-  if(alpm_option_get_logmask() & PM_LOG_DEBUG) {
-		time_t t;
-		struct tm *tmp;
-		char timestr[10] = {0};
-
-		t = time(NULL);
-		tmp = localtime(&t);
-		strftime(timestr, 9, "%H:%M:%S", tmp);
-		timestr[8] = '\0';
-
-		printf("[%s] %s: %s\n", timestr, str, msg);
-	} else {
-    printf("%s: %s\n", str, msg);
-	}
-#else
-	printf("%s: %s\n", str, msg);
-#endif
+	pm_vfprintf(stdout, level, fmt, args);
 }
 
 /* vim: set ts=2 sw=2 noet: */

src/pacman/callback.h

@@ -38,7 +38,7 @@ void cb_trans_progress(pmtransprog_t event, const char *pkgname, int percent,
 void cb_dl_progress(const char *filename, int xfered, int total);
 
 /* callback to handle messages/notifications from pacman library */
-void cb_log(pmloglevel_t level, char *msg);
+void cb_log(pmloglevel_t level, char *fmt, va_list args);
 
 #endif /* _PM_CALLBACK_H */
 

src/util/testpkg.c

@@ -23,20 +23,21 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <stdarg.h>
 #include <string.h>
 #include <libgen.h>
 
 #include <alpm.h>
 
-void output_cb(pmloglevel_t level, char *msg)
+void output_cb(pmloglevel_t level, char *fmt, va_list args)
 {
-	if(strlen(msg)) {
+	if(strlen(fmt)) {
         switch(level) {
         case PM_LOG_ERROR: printf("error: "); break;
         case PM_LOG_WARNING: printf("warning: "); break;
 		default: break;
         }
-        puts(msg);
+		vprintf(fmt, args);
     }
 }