Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

zowelog zss format handler #126

Open
wants to merge 5 commits into
base: v1.x/staging
Choose a base branch
from
Open

zowelog zss format handler #126

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9587670
WIP zowelog zss format handler
sakeerthy Feb 27, 2020
6b3f1c6
removed code
sakeerthy Feb 27, 2020
98252ff
Added constants for component names
sakeerthy Feb 28, 2020
994754b
Removed unneeded passed args
sakeerthy Feb 28, 2020
e74f43c
Removed old zowelog/zowedump
sakeerthy Feb 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions c/crossmemory.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ void wtoPrintf2(int consoleID, CART cart, char *formatString, ...) {

}

static void printWithPrefix(LoggingContext *context, LoggingComponent *component, void *data, char *formatString, va_list argList);
static void printWithPrefix(LoggingContext *context, LoggingComponent *component, char* path, int line, int level, uint64 compID, void *data, char *formatString, va_list argList);
static char *dumpWithEmptyMessageID(char *workBuffer, int workBufferSize, void *data, int dataSize, int lineNumber);

void cmsInitializeLogging() {
Expand Down Expand Up @@ -503,7 +503,7 @@ static void initLogMessagePrefix(LogMessagePrefix *prefix) {
#define PREFIXED_LINE_MAX_COUNT 1000
#define PREFIXED_LINE_MAX_MSG_LENGTH 4096

static void printWithPrefix(LoggingContext *context, LoggingComponent *component, void *data, char *formatString, va_list argList) {
static void printWithPrefix(LoggingContext *context, LoggingComponent *component, char* path, int line, int level, uint64 compID, void *data, char *formatString, va_list argList) {

char messageBuffer[PREFIXED_LINE_MAX_MSG_LENGTH];
size_t messageLength = vsnprintf(messageBuffer, sizeof(messageBuffer), formatString, argList);
Expand Down Expand Up @@ -2808,12 +2808,13 @@ static void printDisplayConfigCommandResponse(CrossMemoryServer *server, CMSWTOR
int consoleID = routeInfo->consoleID;
CrossMemoryServerGlobalArea *globalArea = server->globalArea;

zowelog(NULL, LOG_COMP_ID_CMS, ZOWE_LOG_INFO,
#ifdef CROSS_MEMORY_SERVER_DEBUG
CMS_LOG_DISP_CMD_RESULT_MSG"Server name - \'%16.16s\' (debug mode)\n"
char* formatStringPrefix = CMS_LOG_DISP_CMD_RESULT_MSG"Server name - \'%16.16s\' (debug mode)\n";
#else
CMS_LOG_DISP_CMD_RESULT_MSG"Server name - \'%16.16s\'\n"
char* formatStringPrefix = CMS_LOG_DISP_CMD_RESULT_MSG"Server name - \'%16.16s\'\n";
#endif

zowelog(NULL, LOG_COMP_ID_CMS, ZOWE_LOG_INFO, strcat(formatStringPrefix,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you strcat into a string literal? Have you tested this code? I would expect an S0C4.

Copy link
Contributor

@daveyc daveyc Mar 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed! it's a buffer overflow into the literal pool! Unfortunately, it may not ABEND, it will just clobber the literal pool. strcat is a code smell and should never be used.

" Global area address = 0x%p\n"
" Version = %d\n"
" Key = %d\n"
Expand All @@ -2828,7 +2829,7 @@ static void printDisplayConfigCommandResponse(CrossMemoryServer *server, CMSWTOR
" PC-ss seq number = 0x%08X\n"
" PC-cp PC number = 0x%08X\n"
" PC-cp seq number = 0x%08X\n"
" PC log level = %d\n",
" PC log level = %d\n"),
server->name.nameSpacePadded,
globalArea,
globalArea->version,
Expand Down
26 changes: 18 additions & 8 deletions c/logging.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
#include "logging.h"
#include "printables_for_dump.h"

#include "timeutls.h"
#include "zos.h"
#include "openprims.h"
#include "zssLogging.h"

#ifdef __ZOWE_OS_ZOS
#include "le.h"
#endif
Expand Down Expand Up @@ -311,7 +316,7 @@ static void lastResortLog(char *s){
}

/* default dumper, based on dumpBufferToStream from utils.c */
static char *standardDumperFunction(char *workBuffer, int workBufferSize,
char *standardDumperFunction(char *workBuffer, int workBufferSize,
void *data, int dataSize, int lineNumber){

int formatWidth = 32;
Expand Down Expand Up @@ -455,15 +460,15 @@ LoggingDestination *logConfigureDestination2(LoggingContext *context,
return destination;
}

void printStdout(LoggingContext *context, LoggingComponent *component, void *data, char *formatString, va_list argList){
void printStdout(LoggingContext *context, LoggingComponent *component, char* path, int line, int level, uint64 compID, void *data, char *formatString, va_list argList){
#ifdef METTLE
printf("broken printf in logging.c\n");
#else
vfprintf(stdout,formatString,argList);
#endif
}

void printStderr(LoggingContext *context, LoggingComponent *component, void *data, char *formatString, va_list argList){
void printStderr(LoggingContext *context, LoggingComponent *component, char* path, int line, int level, uint64 compID, void *data, char *formatString, va_list argList){
#ifdef METTLE
printf("broken printf in logging.c\n");
#else
Expand Down Expand Up @@ -647,7 +652,10 @@ int logGetLevel(LoggingContext *context, uint64 compID){
return component ? component->currentDetailLevel : ZOWE_LOG_NA;
}

void zowelog(LoggingContext *context, uint64 compID, int level, char *formatString, ...){


void _zowelog(LoggingContext *context, uint64 compID, char* path, int line, int level, char *formatString, ...){
//void zowelog(LoggingContext *context, uint64 compID, int level, char *formatString, ...){

if (logShouldTrace(context, compID, level) == FALSE) {
return;
Expand Down Expand Up @@ -688,7 +696,7 @@ void zowelog(LoggingContext *context, uint64 compID, int level, char *formatStri
/* here, pass to a var-args handler */
va_start(argPointer, formatString);

destination->handler(context,component,destination->data,formatString,argPointer);
destination->handler(context,component,path,line,level,compID,destination->data,formatString,argPointer);

va_end(argPointer);
}
Expand All @@ -698,14 +706,16 @@ void zowelog(LoggingContext *context, uint64 compID, int level, char *formatStri
static void printToDestination(LoggingDestination *destination,
struct LoggingContext_tag *context,
LoggingComponent *component,
char* path, int line,
int level, uint64 compID,
void *data, char *formatString, ...){
va_list argPointer;
va_start(argPointer, formatString);
destination->handler(context,component,destination->data,formatString,argPointer);
destination->handler(context,component,path,line,level,compID,destination->data,formatString,argPointer);
va_end(argPointer);
}

void zowedump(LoggingContext *context, uint64 compID, int level, void *data, int dataSize){
void _zowedump(LoggingContext *context, uint64 compID, int level, void *data, int dataSize, char* path, int line){

if (logShouldTrace(context, compID, level) == FALSE) {
return;
Expand Down Expand Up @@ -746,7 +756,7 @@ void zowedump(LoggingContext *context, uint64 compID, int level, void *data, int
for (int i = 0; ; i++){
char *result = destination->dumper(workBuffer, sizeof(workBuffer), data, dataSize, i);
if (result != NULL){
printToDestination(destination, context, component, destination->data, "%s\n", result);
printToDestination(destination, context, component, path, line, level, compID, destination->data, "%s\n", result);
}
else {
break;
Expand Down
40 changes: 34 additions & 6 deletions h/logging.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,14 +93,34 @@ ZOWE_PRAGMA_PACK_RESET
#define LOG_COMP_RESTDATASET 0x008F0001000E0000LLU
#define LOG_COMP_RESTFILE 0x008F0001000F0000LLU


#define LOG_COMP_TEXT_ALLOC "alloc"
#define LOG_COMP_TEXT_UTILS "utils"
#define LOG_COMP_TEXT_COLLECTIONS "collections"
#define LOG_COMP_TEXT_SERIALIZATION "serialization"
#define LOG_COMP_TEXT_ZLPARSER "zlparser"
#define LOG_COMP_TEXT_ZLCOMPILER "zlcompiler"
#define LOG_COMP_TEXT_ZLRUNTIME "zlruntime"
#define LOG_COMP_TEXT_STCBASE "stcbase"
#define LOG_COMP_TEXT_HTTPSERVER "httpserver"
#define LOG_COMP_TEXT_DISCOVERY "discovery"
#define LOG_COMP_TEXT_DATASERVICE "dataservice"
#define LOG_COMP_TEXT_CMS "cms"
#define LOG_COMP_TEXT_LPA "lpa"
#define LOG_COMP_TEXT_RESTDATASET "restdataset"
#define LOG_COMP_TEXT_RESTFILE "restfile"

#define LOG_DEST_DEV_NULL 0x008F0000
#define LOG_DEST_PRINTF_STDOUT 0x008F0001
#define LOG_DEST_PRINTF_STDERR 0x008F0002

struct LoggingContext_tag;

typedef void (*LogHandler)(struct LoggingContext_tag *context,
LoggingComponent *component,
LoggingComponent *component,
Copy link
Contributor

@ifakhrutdinov ifakhrutdinov Mar 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks all the code that uses logging, we shouldn't make such changes in minor releases. If you kept compatibility you wouldn't have to change the cross memory code. Imagine that someone uses this API, do they now need to change their code?

What about binaries? You will break existing plugins because you'll call theirs handler assuming it's something else.

Please reconsider this design. You may want to introduce another handler type, allow new code to set it and check at runtime whether you need to caller the old handler type or the new one. That way you don't have to change so much code and break old existing code.

Same applies to the dump function.

char* path, int line,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not mix tabs and spaces. This applies to all the changes in this PR.

int level,
uint64 compID,
void *data,
char *formatString,
va_list argList);
Expand Down Expand Up @@ -297,15 +317,23 @@ bool logShouldTraceInternal(LoggingContext *context, uint64 componentID, int lev

/* this log message will be sent to the destination associated to the component
*/
void _zowelog(LoggingContext *context, uint64 compID, char* path, int line, int level, char *formatString, ...);
void _zowedump(LoggingContext *context, uint64 compID, int level, void *data, int dataSize, char* path, int line);

void zowelog(LoggingContext *context, uint64 compID, int level, char *formatString, ...);
void zowedump(LoggingContext *context, uint64 compID, int level, void *data, int dataSize);

char *standardDumperFunction(char *workBuffer, int workBufferSize,
void *data, int dataSize, int lineNumber);
#define LOGCHECK(context,component,level) \
((component > MAX_LOGGING_COMPONENTS) ? \
(context->applicationComponents[component].level >= level) : \
(context->coreComponents[component].level >= level) )


#define zowelog(context, compID, level, formatString, ...) \
_zowelog(context, compID, __FILE__, __LINE__, level, formatString, ##__VA_ARGS__);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By introducing a macro we will break existing plugins because they assume that zowelog has fewer args. It may be better to have zowelog2 to keep the backward compatibility.


#define zowedump(context, compID, level, data, dataSize) \
_zowedump(context, compID, level, data, dataSize, __FILE__, __LINE__);

LoggingDestination *logConfigureDestination(LoggingContext *context,
unsigned int id,
char *name,
Expand All @@ -332,8 +360,8 @@ int logGetLevel(LoggingContext *context, uint64 compID);
extern int logSetExternalContext(LoggingContext *context);
extern LoggingContext *logGetExternalContext();

void printStdout(LoggingContext *context, LoggingComponent *component, void *data, char *formatString, va_list argList);
void printStderr(LoggingContext *context, LoggingComponent *component, void *data, char *formatString, va_list argList);
void printStdout(LoggingContext *context, LoggingComponent *component, char* path, int line, int level, uint64 compID, void *data, char *formatString, va_list argList);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not change existing APIs.

void printStderr(LoggingContext *context, LoggingComponent *component, char* path, int line, int level, uint64 compID, void *data, char *formatString, va_list argList);

#endif

Expand Down