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

diff --git a/include/opensrf/log.h b/include/opensrf/log.h
index 4aafad3..9096d86 100644 (file)
--- 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 (file)
--- a/src/libopensrf/log.c
+++ b/src/libopensrf/log.c
@@ -1,6 +1,6 @@
 #include <opensrf/log.h>
 
-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));
 
 }