From 442fe6e7a73150f2a06d2fc6760939aee38aa983 Mon Sep 17 00:00:00 2001 From: miker Date: Mon, 25 Jun 2007 03:06:56 +0000 Subject: [PATCH] Patch from Scott McKellar to improve default logging: 1. In log.h I added another macro OSRF_LOG_TYPE_STDERR by which the application can specifically request logging to standard error. 2. In log.c I initialze _osrfLogType to OSRF_LOG_TYPE_STDERR so that the application can issue a log message even if it hasn't called osrfLogInit() yet. 3. I added the const qualifier to the parameters of the functions _osrfLogToFile() and _osrfLogSetXid(). 4. In osrfLogCleanup() I set _osrfLogAppname and _osrfLogFile to NULL after freeing them. Otherwise the application could cause needless mischief by calling this function twice. 5. Also in osrfLogCleanup() I set _osrfLogType back to the default OSRF_LOG_TYPE_STDERR so that the application can still issue a message after calling osrfLogCleanup(). 6. I rewrote osrfLogSetType() to use switch/case instead of an "if". Now that we have three valid values to check instead of two, a switch/case is a little tidier. Also the default branch applies the default log type OSRF_LOG_TYPE_STDERR. 7. _osrfLogDetail() had a local variable named "l" (lower case L). I renamed it to "label" because "l" looks too much like a digit. 8.Also in _osrfLogDetail(): I added a branch to write a message to stderr when _osrfLogType is OSRF_LOG_TYPE_STDERR. 9. The existing _osrfLogToFile function sizes and declares a buffer named "buf", fills it with zeros, and then doesn't do anything with it. This buffer is probably a relic of some earlier version. I eliminated it. 10. A few lines thereafter, I eliminated a useless call to bzero(). 11. The call to strftime() contained a hard-coded buffer size. I changed it to use the sizeof operator, so that the buffer size is defined in only one place. 12. If we can't open the log file, we write a message to stderr. In this message I changed "file" to "log file" to make the meaning more clear to a benighted and panicky user. 13. If after writing the message we can't close the file, the existing code logs an error message by calling osrfLogWarning(). However in this circumstance this latter message will probably meet the same fate, and no one will ever see it. I changed the code so as to write this message to stderr. git-svn-id: svn://svn.open-ils.org/OpenSRF/trunk@974 9efc2488-bf62-4759-914b-345cdb29e865 --- include/opensrf/log.h | 1 + src/libopensrf/log.c | 62 ++++++++++++++++++++++++------------------- 2 files changed, 36 insertions(+), 27 deletions(-) diff --git a/include/opensrf/log.h b/include/opensrf/log.h index 4aafad3..9096d86 100644 --- a/include/opensrf/log.h +++ b/include/opensrf/log.h @@ -18,6 +18,7 @@ #define OSRF_LOG_TYPE_FILE 1 #define OSRF_LOG_TYPE_SYSLOG 2 +#define OSRF_LOG_TYPE_STDERR 3 #define OSRF_LOG_MARK __FILE__, __LINE__ diff --git a/src/libopensrf/log.c b/src/libopensrf/log.c index 35d2756..92023ae 100644 --- a/src/libopensrf/log.c +++ b/src/libopensrf/log.c @@ -1,6 +1,6 @@ #include -static int _osrfLogType = -1; +static int _osrfLogType = OSRF_LOG_TYPE_STDERR; static int _osrfLogFacility = LOG_LOCAL0; static int _osrfLogActFacility = LOG_LOCAL1; static char* _osrfLogFile = NULL; @@ -14,8 +14,8 @@ static char* _osrfLogXidPfx = NULL; /* xid prefix string */ static void osrfLogSetType( int logtype ); static void _osrfLogDetail( int level, const char* filename, int line, char* msg ); -static void _osrfLogToFile( char* msg, ... ); -static void _osrfLogSetXid(char* xid); +static void _osrfLogToFile( const char* msg, ... ); +static void _osrfLogSetXid( const char* xid ); #define OSRF_LOG_GO(f,li,m,l) \ if(!m) return; \ @@ -24,7 +24,10 @@ static void _osrfLogSetXid(char* xid); void osrfLogCleanup() { free(_osrfLogAppname); + _osrfLogAppname = NULL; free(_osrfLogFile); + _osrfLogFile = NULL; + _osrfLogType = OSRF_LOG_TYPE_STDERR; } @@ -36,7 +39,7 @@ void osrfLogInit( int type, const char* appname, int maxlevel ) { openlog(_osrfLogAppname, 0, _osrfLogFacility ); } -static void _osrfLogSetXid(char* xid) { +static void _osrfLogSetXid( const char* xid ) { if(xid) { if(_osrfLogXid) free(_osrfLogXid); _osrfLogXid = strdup(xid); @@ -74,13 +77,20 @@ void osrfLogSetIsClient(int is) { } /** Sets the type of logging to perform. See log types */ -static void osrfLogSetType( int logtype ) { - if( logtype != OSRF_LOG_TYPE_FILE && - logtype != OSRF_LOG_TYPE_SYSLOG ) { - fprintf(stderr, "Unrecognized log type. Logging to stderr\n"); - return; +static void osrfLogSetType( int logtype ) { + + switch( logtype ) + { + case OSRF_LOG_TYPE_FILE : + case OSRF_LOG_TYPE_SYSLOG : + case OSRF_LOG_TYPE_STDERR : + _osrfLogType = logtype; + break; + default : + fprintf(stderr, "Unrecognized log type. Logging to stderr\n"); + _osrfLogType = OSRF_LOG_TYPE_STDERR; + break; } - _osrfLogType = logtype; } void osrfLogSetFile( const char* logfile ) { @@ -146,38 +156,38 @@ static void _osrfLogDetail( int level, const char* filename, int line, char* msg if(!msg) return; if(!filename) filename = ""; - char* l = "INFO"; /* level name */ + char* label = "INFO"; /* level name */ int lvl = LOG_INFO; /* syslog level */ int fac = _osrfLogFacility; switch( level ) { case OSRF_LOG_ERROR: - l = "ERR "; + label = "ERR "; lvl = LOG_ERR; break; case OSRF_LOG_WARNING: - l = "WARN"; + label = "WARN"; lvl = LOG_WARNING; break; case OSRF_LOG_INFO: - l = "INFO"; + label = "INFO"; lvl = LOG_INFO; break; case OSRF_LOG_DEBUG: - l = "DEBG"; + label = "DEBG"; lvl = LOG_DEBUG; break; case OSRF_LOG_INTERNAL: - l = "INT "; + label = "INT "; lvl = LOG_DEBUG; break; case OSRF_LOG_ACTIVITY: - l = "ACT"; + label = "ACT"; lvl = LOG_INFO; fac = _osrfLogActFacility; break; @@ -194,41 +204,39 @@ static void _osrfLogDetail( int level, const char* filename, int line, char* msg buf[1533] = '.'; buf[1534] = '.'; buf[1535] = '\0'; - syslog( fac | lvl, "[%s:%ld:%s:%d:%s] %s", l, (long) getpid(), filename, line, xid, buf ); + syslog( fac | lvl, "[%s:%ld:%s:%d:%s] %s", label, (long) getpid(), filename, line, xid, buf ); } else if( _osrfLogType == OSRF_LOG_TYPE_FILE ) - _osrfLogToFile("[%s:%ld:%s:%d:%s] %s", l, (long) getpid(), filename, line, xid, msg ); + _osrfLogToFile( "[%s:%ld:%s:%d:%s] %s", label, (long) getpid(), filename, line, xid, msg ); + else if( _osrfLogType == OSRF_LOG_TYPE_STDERR ) + fprintf( stderr, "[%s:%ld:%s:%d:%s] %s\n", label, (long) getpid(), filename, line, xid, msg ); } -static void _osrfLogToFile( char* msg, ... ) { +static void _osrfLogToFile( const char* msg, ... ) { if(!msg) return; if(!_osrfLogFile) return; VA_LIST_TO_STRING(msg); if(!_osrfLogAppname) _osrfLogAppname = strdup("osrf"); - int l = strlen(VA_BUF) + strlen(_osrfLogAppname) + 36; - char buf[l]; - bzero(buf,l); char datebuf[36]; - bzero(datebuf,36); time_t t = time(NULL); struct tm* tms = localtime(&t); - strftime(datebuf, 36, "%Y-%m-%d %H:%M:%S", tms); + strftime(datebuf, sizeof( datebuf ), "%Y-%m-%d %H:%M:%S", tms); FILE* file = fopen(_osrfLogFile, "a"); if(!file) { - fprintf(stderr, "Unable to fopen file %s for writing\n", _osrfLogFile); + fprintf(stderr, "Unable to fopen log file %s for writing\n", _osrfLogFile); return; } fprintf(file, "%s %s %s\n", _osrfLogAppname, datebuf, VA_BUF ); if( fclose(file) != 0 ) - osrfLogWarning(OSRF_LOG_MARK, "Error closing log file: %s", strerror(errno)); + fprintf( stderr, "Error closing log file: %s", strerror(errno)); } -- 2.43.2