From 0cb7c9aafe4bd16a743f9c982760638c9f7032ee Mon Sep 17 00:00:00 2001 From: Murali Shankar Date: Tue, 23 Aug 2011 11:04:09 -0700 Subject: [PATCH 1/5] Added support for iocLogPrefix Refactored logClientSend in libCom/logClient/logClient.c; took the code between the mutex operations and moved it to a private method - sendLogMessageinChunks. Call this method once for the prefix (if it exists) and once for the actual message. Added ioCsh registration code into src/libCom/iocsh/libComRegister.c registering a command called "iocLogPrefix" that sets this prefix. Unit tested with and without prefixes. Performance tested with and without prefixes - without prefix is approx the same. With prefix is about twice the time (reflecting the two calls to sendLogMessageinChunks I think) --- src/libCom/iocsh/libComRegister.c | 10 +++++ src/libCom/logClient/logClient.c | 66 +++++++++++++++++++++++++------ 2 files changed, 64 insertions(+), 12 deletions(-) diff --git a/src/libCom/iocsh/libComRegister.c b/src/libCom/iocsh/libComRegister.c index 6cd79bb66..f6ce30284 100644 --- a/src/libCom/iocsh/libComRegister.c +++ b/src/libCom/iocsh/libComRegister.c @@ -191,6 +191,15 @@ static void errlogCallFunc(const iocshArgBuf *args) errlogPrintfNoConsole("%s\n", args[0].sval); } +/* iocLogPrefix */ +static const iocshArg iocLogPrefixArg0 = { "prefix",iocshArgString}; +static const iocshArg * const iocLogPrefixArgs[1] = {&iocLogPrefixArg0}; +static const iocshFuncDef iocLogPrefixFuncDef = {"iocLogPrefix",1,iocLogPrefixArgs}; +static void iocLogPrefixCallFunc(const iocshArgBuf *args) +{ + iocLogPrefix(args[0].sval); +} + /* epicsThreadShowAll */ static const iocshArg epicsThreadShowAllArg0 = { "level",iocshArgInt}; static const iocshArg * const epicsThreadShowAllArgs[1] = {&epicsThreadShowAllArg0}; @@ -355,6 +364,7 @@ void epicsShareAPI libComRegister(void) iocshRegister(&errlogInitFuncDef,errlogInitCallFunc); iocshRegister(&errlogInit2FuncDef,errlogInit2CallFunc); iocshRegister(&errlogFuncDef, errlogCallFunc); + iocshRegister(&iocLogPrefixFuncDef, iocLogPrefixCallFunc); iocshRegister(&epicsThreadShowAllFuncDef,epicsThreadShowAllCallFunc); iocshRegister(&threadFuncDef, threadCallFunc); diff --git a/src/libCom/logClient/logClient.c b/src/libCom/logClient/logClient.c index adaaa7f95..1c2e796f1 100644 --- a/src/libCom/logClient/logClient.c +++ b/src/libCom/logClient/logClient.c @@ -57,6 +57,11 @@ static const double LOG_RESTART_DELAY = 5.0; /* sec */ static const double LOG_SERVER_CREATE_CONNECT_SYNC_TIMEOUT = 5.0; /* sec */ static const double LOG_SERVER_SHUTDOWN_TIMEOUT = 30.0; /* sec */ +/* + * The logClientPrefix stores a prefix that is sent as a prefix for all log messages. + */ +static char* logClientPrefix = NULL; + /* * logClientClose () */ @@ -160,21 +165,13 @@ static void logClientDestroy (logClientId id) } /* - * logClientSend () + * private method with code refactored out of logClientSend. + * This method relies on the mutex being obtained on pClient->mutex (which happens in logClientSend prior to this method being called) */ -void epicsShareAPI logClientSend ( logClientId id, const char * message ) -{ - logClient * pClient = ( logClient * ) id; +static void sendLogMessageinChunks(logClient * pClient, const char * message) { unsigned strSize; - if ( ! pClient || ! message ) { - return; - } - strSize = strlen ( message ); - - epicsMutexMustLock ( pClient->mutex ); - while ( strSize ) { unsigned msgBufBytesLeft = sizeof ( pClient->msgBuf ) - pClient->nextMsgIndex; @@ -231,10 +228,31 @@ void epicsShareAPI logClientSend ( logClientId id, const char * message ) break; } } - +} + + +/* + * logClientSend () + */ +void epicsShareAPI logClientSend ( logClientId id, const char * message ) +{ + logClient * pClient = ( logClient * ) id; + + if ( ! pClient || ! message ) { + return; + } + + epicsMutexMustLock ( pClient->mutex ); + + if(logClientPrefix) { + sendLogMessageinChunks(pClient, logClientPrefix); + } + sendLogMessageinChunks(pClient, message); + epicsMutexUnlock (pClient->mutex); } + void epicsShareAPI logClientFlush ( logClientId id ) { logClient * pClient = ( logClient * ) id; @@ -555,3 +573,27 @@ void epicsShareAPI logClientShow (logClientId id, unsigned level) } } +/* + * iocLogPrefix() + **/ +int epicsShareAPI iocLogPrefix(const char* prefix) +{ + // If we have already established a log prefix, free it + // Note iocLogPrefix is expected to be set in the cmd file during initialization. + // We do not anticipate changing this after it has been set + if(logClientPrefix) { + free(logClientPrefix); + logClientPrefix = NULL; + } + + if(prefix) { + unsigned prefixLen = strlen(prefix); + if(prefixLen > 0) { + char* localCopy = malloc(prefixLen+1); + strcpy(localCopy, prefix); + logClientPrefix = localCopy; + } + } +} + + From 5a0364b74ae45b95407ed762f0c32ec380365f8c Mon Sep 17 00:00:00 2001 From: Murali Shankar Date: Tue, 23 Aug 2011 11:51:19 -0700 Subject: [PATCH 2/5] iocLogPrefix does not let you change the prefix if the prefix has already been set. Display the prefix as part of iocLogShow. --- src/libCom/logClient/logClient.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/libCom/logClient/logClient.c b/src/libCom/logClient/logClient.c index 1c2e796f1..f86b216ef 100644 --- a/src/libCom/logClient/logClient.c +++ b/src/libCom/logClient/logClient.c @@ -571,19 +571,23 @@ void epicsShareAPI logClientShow (logClientId id, unsigned level) pClient->sock==INVALID_SOCKET?"INVALID":"OK", pClient->connectCount); } + + if(logClientPrefix) { + printf ("log client: a log prefix has been set \"%s\"\n", logClientPrefix); + } } /* * iocLogPrefix() **/ -int epicsShareAPI iocLogPrefix(const char* prefix) +void epicsShareAPI iocLogPrefix(const char* prefix) { - // If we have already established a log prefix, free it + // If we have already established a log prefix, do not let the user change it // Note iocLogPrefix is expected to be set in the cmd file during initialization. // We do not anticipate changing this after it has been set if(logClientPrefix) { - free(logClientPrefix); - logClientPrefix = NULL; + printf ("log client: a log prefix has already been established \"%s\". Ignoring this call \n", logClientPrefix); + return; } if(prefix) { From 0a4daefee3e6078b0df5459b11dc0f58d8aebda3 Mon Sep 17 00:00:00 2001 From: Murali Shankar Date: Wed, 24 Aug 2011 11:24:26 -0700 Subject: [PATCH 3/5] Added the iocLogPrefix to the header so that we can use it in the unit test --- src/libCom/logClient/logClient.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libCom/logClient/logClient.h b/src/libCom/logClient/logClient.h index 8f1ec09d5..1797bbb20 100644 --- a/src/libCom/logClient/logClient.h +++ b/src/libCom/logClient/logClient.h @@ -33,6 +33,7 @@ epicsShareFunc logClientId epicsShareAPI logClientCreate ( epicsShareFunc void epicsShareAPI logClientSend (logClientId id, const char *message); epicsShareFunc void epicsShareAPI logClientShow (logClientId id, unsigned level); epicsShareFunc void epicsShareAPI logClientFlush (logClientId id); +epicsShareFunc void epicsShareAPI iocLogPrefix(const char* prefix); /* deprecated interface; retained for backward compatibility */ /* note: implementations are in iocLog.c, not logClient.c */ From c12f6808f1a7bd78bfbfcae7f96f7b09cc12103f Mon Sep 17 00:00:00 2001 From: Murali Shankar Date: Wed, 24 Aug 2011 11:25:06 -0700 Subject: [PATCH 4/5] Added a couple of units tests for iocLogPrefix --- src/libCom/test/epicsErrlogTest.c | 166 +++++++++++++++++++++++++++++- 1 file changed, 165 insertions(+), 1 deletion(-) diff --git a/src/libCom/test/epicsErrlogTest.c b/src/libCom/test/epicsErrlogTest.c index 08389e438..78729d7ad 100644 --- a/src/libCom/test/epicsErrlogTest.c +++ b/src/libCom/test/epicsErrlogTest.c @@ -22,6 +22,11 @@ #include "errlog.h" #include "epicsUnitTest.h" #include "testMain.h" +#include "iocLog.h" +#include "logClient.h" +#include "envDefs.h" +#include "osiSock.h" +#include "fdmgr.h" #define LOGBUFSIZE 2048 @@ -75,6 +80,27 @@ typedef struct { int jam; } clientPvt; +static void testLogPrefix(void); +static void acceptNewClient( void *pParam ); +static void readFromClient( void *pParam ); +static void testPrefixLogandCompare( const char* logmessage); + +static void *pfdctx; +static SOCKET sock; +static SOCKET insock; + +static const char* prefixactualmsg[]= { + "A message without prefix", + "A message with prefix", + "DONE" + }; +static const char prefixexpectedmsg[] = "A message without prefix" + "fac=LI21 A message with prefix" + "fac=LI21 DONE" + ; +static char prefixmsgbuffer[1024]; + + static void logClient(void* raw, const char* msg) { @@ -115,7 +141,7 @@ MAIN(epicsErrlogTest) char msg[256]; clientPvt pvt, pvt2; - testPlan(25); + testPlan(35); strcpy(msg, truncmsg); @@ -289,5 +315,143 @@ MAIN(epicsErrlogTest) /* Clean up */ errlogRemoveListener(&logClient); + testLogPrefix(); + return testDone(); } +/* + * Tests the log prefix code + * Since the log prefix is only applied to log messages as they are going out on the socket, + * we need to create a server listening on a port, accept connections etc. + * This code is a reduced version of the code in iocLogServer. + */ +static void testLogPrefix(void) { + struct sockaddr_in serverAddr; + int status; + struct timeval timeout; + struct sockaddr_in actualServerAddr; + osiSocklen_t actualServerAddrSize; + + + testDiag("Testing iocLogPrefix"); + + timeout.tv_sec = 5; /* in seconds */ + timeout.tv_usec = 0; + + memset((void*)prefixmsgbuffer, 0, sizeof prefixmsgbuffer); + + /* Clear "errlog: messages were discarded" status */ + errlogPrintfNoConsole("."); + errlogFlush(); + + sock = epicsSocketCreate(AF_INET, SOCK_STREAM, 0); + testOk1(sock != INVALID_SOCKET); + + // We listen on a an available port. + memset((void *)&serverAddr, 0, sizeof serverAddr); + serverAddr.sin_family = AF_INET; + serverAddr.sin_port = htons(0); + + status = bind (sock, + (struct sockaddr *)&serverAddr, + sizeof (serverAddr) ); + testOk1(status >= 0); + + status = listen(sock, 10); + testOk1(status >= 0); + + // Determine the port that the OS chose + actualServerAddrSize = sizeof actualServerAddr; + memset((void *)&actualServerAddr, 0, sizeof serverAddr); + status = getsockname(sock, (struct sockaddr *) &actualServerAddr, &actualServerAddrSize); + testOk1(status >= 0); + + char portstring[16]; + sprintf(portstring, "%d", ntohs(actualServerAddr.sin_port)); + testDiag("Listening on port %s", portstring); + + // Set the EPICS environment variables for logging. + epicsEnvSet ( "EPICS_IOC_LOG_INET", "localhost" ); + epicsEnvSet ( "EPICS_IOC_LOG_PORT", portstring ); + + pfdctx = (void *) fdmgr_init(); + testOk1(pfdctx != NULL); + + status = fdmgr_add_callback( + pfdctx, + sock, + fdi_read, + acceptNewClient, + &serverAddr); + testOk1(status >= 0); + + status = iocLogInit (); + testOk1(status >= 0); + fdmgr_pend_event(pfdctx, &timeout); + + testPrefixLogandCompare(prefixactualmsg[0]); + + iocLogPrefix("fac=LI21 "); + testPrefixLogandCompare(prefixactualmsg[1]); + testPrefixLogandCompare(prefixactualmsg[2]); + + close(sock); +} + +static void testPrefixLogandCompare( const char* logmessage ) { + struct timeval timeout; + timeout.tv_sec = 5; /* in seconds */ + timeout.tv_usec = 0; + + errlogPrintfNoConsole(logmessage); + errlogFlush(); + iocLogFlush(); + fdmgr_pend_event(pfdctx, &timeout); +} + +static void acceptNewClient ( void *pParam ) +{ + osiSocklen_t addrSize; + struct sockaddr_in addr; + int status; + + addrSize = sizeof ( addr ); + insock = epicsSocketAccept ( sock, (struct sockaddr *)&addr, &addrSize ); + testOk1(insock!=INVALID_SOCKET && addrSize >= sizeof (addr) ); + + status = fdmgr_add_callback( + pfdctx, + insock, + fdi_read, + readFromClient, + NULL); + testOk1(status >= 0); +} + +static void readFromClient(void *pParam) +{ + char recvbuf[1024]; + int recvLength; + + memset(&recvbuf, 0, 1024); + recvLength = recv(insock, + &recvbuf, + 1024, + 0); + if (recvLength > 0) { + strcat(prefixmsgbuffer, recvbuf); + + // If we have received all of the messages. + if(strstr(prefixmsgbuffer, "DONE") != NULL) { + int prefixcmp = strncmp(prefixexpectedmsg, prefixmsgbuffer, strlen(prefixexpectedmsg)); + if(prefixcmp != 0) { + printf("Expected %s\n", prefixexpectedmsg); + printf("Obtained %s\n", prefixmsgbuffer); + } + + testOk1(prefixcmp == 0); + } + } +} + + From 95eb4790f12c13f17519a1df556c1efb5558e36a Mon Sep 17 00:00:00 2001 From: Murali Shankar Date: Wed, 24 Aug 2011 11:25:37 -0700 Subject: [PATCH 5/5] Added iocLogPrefix to the release notes --- documentation/RELEASE_NOTES.html | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/documentation/RELEASE_NOTES.html b/documentation/RELEASE_NOTES.html index 5e46fc357..cc4af9e3e 100644 --- a/documentation/RELEASE_NOTES.html +++ b/documentation/RELEASE_NOTES.html @@ -35,5 +35,15 @@ Removed 3.13 compatibility Removed the 3.13 <top>/config directory and build compatibility rules and variables, and various conversion documents.

+

+Added support for iocLogPrefix

+ +

+Added a iocLogPrefix command to ioCsh. This establishes a prefix that will be +common to all log messages as they are sent to the iocLogServer. This lets us use the +"fac=<facility>" syntax for displaying the facility, process name etc. in log viewers like the +cmlogviewer. +

+