From 3a4f679c3b21634ae42534b255d4de0d4f1e4543 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Mon, 28 Apr 2008 09:49:07 +0200 Subject: perparing for klog debug ... and some cleanup --- configure.ac | 2 +- plugins/imklog/ksym.c | 62 ++++++++++++++++++++++++-------------------------- plugins/imklog/linux.c | 3 ++- syslogd.c | 11 ++++----- 4 files changed, 38 insertions(+), 40 deletions(-) diff --git a/configure.ac b/configure.ac index 8a5f6222..0a769092 100644 --- a/configure.ac +++ b/configure.ac @@ -2,7 +2,7 @@ # Process this file with autoconf to produce a configure script. AC_PREREQ(2.61) -AC_INIT([rsyslog],[3.17.2],[rsyslog@lists.adiscon.com]) +AC_INIT([rsyslog],[3.17.2-klog0],[rsyslog@lists.adiscon.com]) AM_INIT_AUTOMAKE AC_CONFIG_SRCDIR([syslogd.c]) AC_CONFIG_HEADERS([config.h]) diff --git a/plugins/imklog/ksym.c b/plugins/imklog/ksym.c index 1c2af124..34ce909e 100644 --- a/plugins/imklog/ksym.c +++ b/plugins/imklog/ksym.c @@ -138,7 +138,7 @@ static char *system_maps[] = /* Function prototypes. */ -static char * FindSymbolFile(void); +static char *FindSymbolFile(void); static int AddSymbol(unsigned long, char*); static void FreeSymbols(void); static int CheckVersion(char *); @@ -173,29 +173,28 @@ extern int InitKsyms(char *mapfile) auto FILE *sym_file; + BEGINfunc /* Check and make sure that we are starting with a clean slate. */ if ( num_syms > 0 ) FreeSymbols(); - /* - * Search for and open the file containing the kernel symbols. - */ - if ( mapfile != (char *) 0 ) { - if ( (sym_file = fopen(mapfile, "r")) == (FILE *) 0 ) + /* Search for and open the file containing the kernel symbols. */ + if ( mapfile != NULL ) { + if ( (sym_file = fopen(mapfile, "r")) == NULL ) { imklogLogIntMsg(LOG_WARNING, "Cannot open map file: %s.", mapfile); return(0); } } else { - if ( (mapfile = FindSymbolFile()) == (char *) 0 ) { + if ( (mapfile = FindSymbolFile()) == NULL ) { imklogLogIntMsg(LOG_WARNING, "Cannot find map file."); dbgprintf("Cannot find map file.\n"); return(0); } - if ( (sym_file = fopen(mapfile, "r")) == (FILE *) 0 ) { + if ( (sym_file = fopen(mapfile, "r")) == NULL ) { imklogLogIntMsg(LOG_WARNING, "Cannot open map file."); dbgprintf("Cannot open map file.\n"); return(0); @@ -248,6 +247,7 @@ extern int InitKsyms(char *mapfile) } fclose(sym_file); + ENDfunc return(1); } @@ -292,44 +292,42 @@ extern void DeinitKsyms(void) **************************************************************************/ static char *FindSymbolFile(void) { - auto char *file = (char *) 0, + auto char *file = NULL, **mf = system_maps; - auto struct utsname utsname; static char mysymfile[100]; + auto FILE *sym_file = NULL; + BEGINfunc - auto FILE *sym_file = (FILE *) 0; - - if ( uname(&utsname) < 0 ) { + if(uname(&utsname) < 0) { imklogLogIntMsg(LOG_ERR, "Cannot get kernel version information."); return(0); } dbgprintf("Searching for symbol map.\n"); - for(mf = system_maps; *mf != (char *) 0 && file == (char *) 0; ++mf) { - + for(mf = system_maps; *mf != NULL && file == NULL; ++mf) { snprintf(mysymfile, sizeof(mysymfile), "%s-%s", *mf, utsname.release); dbgprintf("Trying %s.\n", mysymfile); - if ( (sym_file = fopen(mysymfile, "r")) != (FILE *) 0 ) { - if (CheckMapVersion(mysymfile) == 1) + if((sym_file = fopen(mysymfile, "r")) != NULL) { + if(CheckMapVersion(mysymfile) == 1) file = mysymfile; fclose(sym_file); } - if (sym_file == (FILE *) 0 || file == (char *) 0) { + if(sym_file == NULL || file == NULL) { sprintf (mysymfile, "%s", *mf); dbgprintf("Trying %s.\n", mysymfile); - if ( (sym_file = fopen(mysymfile, "r")) != (FILE *) 0 ) { + if((sym_file = fopen(mysymfile, "r")) != NULL ) { if (CheckMapVersion(mysymfile) == 1) file = mysymfile; fclose(sym_file); } } - } /* At this stage of the game we are at the end of the symbol tables. */ dbgprintf("End of search list encountered.\n"); + ENDfunc return(file); } @@ -464,7 +462,7 @@ static int CheckMapVersion(char *fname) auto char type, sym[512]; - if ( (sym_file = fopen(fname, "r")) != (FILE *) 0 ) { + if ( (sym_file = fopen(fname, "r")) != NULL ) { /* * At this point a map file was successfully opened. We * now need to search this file and look for version @@ -527,7 +525,7 @@ static int AddSymbol(unsigned long address, char *symbol) /* Then the space for the symbol. */ sym_array[num_syms].name = (char *) malloc(strlen(symbol)*sizeof(char) + 1); - if ( sym_array[num_syms].name == (char *) 0 ) + if ( sym_array[num_syms].name == NULL ) return(0); sym_array[num_syms].value = address; @@ -566,13 +564,13 @@ char * LookupSymbol(unsigned long value, struct symbol *sym) struct symbol ksym, msym; if (!sym_array) - return((char *) 0); + return(NULL); last = sym_array[0].name; ksym.offset = 0; ksym.size = 0; if ( value < sym_array[0].value ) - return((char *) 0); + return(NULL); for(lp = 0; lp <= num_syms; ++lp) { if ( sym_array[lp].value > value ) { @@ -587,7 +585,7 @@ char * LookupSymbol(unsigned long value, struct symbol *sym) name = LookupModuleSymbol(value, &msym); if ( ksym.offset == 0 && msym.offset == 0 ) { - return((char *) 0); + return(NULL); } if ( ksym.offset == 0 || msym.offset < 0 || @@ -602,7 +600,7 @@ char * LookupSymbol(unsigned long value, struct symbol *sym) } - return((char *) 0); + return(NULL); } /************************************************************************** @@ -683,7 +681,7 @@ extern char *ExpandKadds(char *line, char *el) * open for patches. */ if ( i_am_paranoid && - (strstr(line, "Oops:") != (char *) 0) && !InitMsyms() ) + (strstr(line, "Oops:") != NULL) && !InitMsyms() ) imklogLogIntMsg(LOG_WARNING, "Cannot load kernel module symbols.\n"); @@ -692,7 +690,7 @@ extern char *ExpandKadds(char *line, char *el) * messages in this line. */ if ( (num_syms == 0) || - (kp = strstr(line, "[<")) == (char *) 0 ) { + (kp = strstr(line, "[<")) == NULL ) { #ifdef __sparc__ if (num_syms) { /* On SPARC, register dumps do not have the [< >] characters in it. @@ -780,7 +778,7 @@ extern char *ExpandKadds(char *line, char *el) *elp++ = *sl++; /* Now poised at a kernel delimiter. */ - if ( (kp = strstr(sl, ">]")) == (char *) 0 ) { + if ( (kp = strstr(sl, ">]")) == NULL ) { strcpy(el, sl); return(el); } @@ -788,7 +786,7 @@ extern char *ExpandKadds(char *line, char *el) strncpy(num,sl+1,kp-sl-1); num[kp-sl-1] = '\0'; value = strtoul(num, (char **) 0, 16); - if ( (symbol = LookupSymbol(value, &sym)) == (char *) 0 ) + if ( (symbol = LookupSymbol(value, &sym)) == NULL ) symbol = sl; strcat(elp, symbol); @@ -805,10 +803,10 @@ extern char *ExpandKadds(char *line, char *el) strncat(elp, kp, value); elp += value; sl = kp + value; - if ( (kp = strstr(sl, "[<")) == (char *) 0 ) + if ( (kp = strstr(sl, "[<")) == NULL ) strcat(elp, sl); } - while ( kp != (char *) 0); + while ( kp != NULL); dbgprintf("Expanded line: %s\n", el); return(el); diff --git a/plugins/imklog/linux.c b/plugins/imklog/linux.c index a742a456..550ea6a1 100644 --- a/plugins/imklog/linux.c +++ b/plugins/imklog/linux.c @@ -504,7 +504,8 @@ rsRetVal klogWillRun(void) symbol_lookup = (InitKsyms(symfile) == 1); symbol_lookup |= InitMsyms(); if (symbol_lookup == 0) { - imklogLogIntMsg(LOG_WARNING, "cannot find any symbols, turning off symbol lookups\n"); + dbgprintf("cannot find any symbols, turning off symbol lookups\n"); + imklogLogIntMsg(LOG_WARNING, "cannot find any symbols, turning off symbol lookups"); } } } diff --git a/syslogd.c b/syslogd.c index bf4f5e67..f4ac2080 100644 --- a/syslogd.c +++ b/syslogd.c @@ -1572,8 +1572,7 @@ submitMsg(msg_t *pMsg) } -/* - * Log a message to the appropriate log files, users, etc. based on +/* Log a message to the appropriate log files, users, etc. based on * the priority. * rgerhards 2004-11-08: actually, this also decodes all but the PRI part. * rgerhards 2004-11-09: ... but only, if syslogd could properly be initialized @@ -2233,12 +2232,12 @@ init(void) tplDeleteNew(); /* re-setting values to defaults (where applicable) */ - /* TODO: once we have loadable modules, we must re-visit this code. The reason is + /* once we have loadable modules, we must re-visit this code. The reason is * that config variables are not re-set, because the module is not yet loaded. On * the other hand, that doesn't matter, because the module got unloaded and is then - * re-loaded, so the variables should be re-set via that way. In any case, we should - * think about the whole situation when we implement loadable plugins. - * rgerhards, 2007-07-31 + * re-loaded, so the variables should be re-set via that way. And this is exactly how + * it works. Loadable module's variables are initialized on load, the rest here. + * rgerhards, 2008-04-28 */ conf.cfsysline((uchar*)"ResetConfigVariables"); -- cgit From a84a60d8dd6e1ec8a04a52be12e62dea81146d5d Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Tue, 29 Apr 2008 10:38:22 +0200 Subject: uncommented dbgprintf's as I think these may be the trouble source this callback is somewhat unportable in combination with dlopen() --- configure.ac | 2 +- plugins/imklog/ksym.c | 35 +++++++++++++++++------------------ plugins/imklog/ksym_mod.c | 2 +- plugins/imklog/linux.c | 6 +++--- 4 files changed, 22 insertions(+), 23 deletions(-) diff --git a/configure.ac b/configure.ac index 0a769092..54824d35 100644 --- a/configure.ac +++ b/configure.ac @@ -2,7 +2,7 @@ # Process this file with autoconf to produce a configure script. AC_PREREQ(2.61) -AC_INIT([rsyslog],[3.17.2-klog0],[rsyslog@lists.adiscon.com]) +AC_INIT([rsyslog],[3.17.2-klog1],[rsyslog@lists.adiscon.com]) AM_INIT_AUTOMAKE AC_CONFIG_SRCDIR([syslogd.c]) AC_CONFIG_HEADERS([config.h]) diff --git a/plugins/imklog/ksym.c b/plugins/imklog/ksym.c index 34ce909e..f02589ce 100644 --- a/plugins/imklog/ksym.c +++ b/plugins/imklog/ksym.c @@ -190,20 +190,19 @@ extern int InitKsyms(char *mapfile) } else { if ( (mapfile = FindSymbolFile()) == NULL ) { imklogLogIntMsg(LOG_WARNING, "Cannot find map file."); - dbgprintf("Cannot find map file.\n"); + ////dbgprintf("Cannot find map file.\n"); return(0); } if ( (sym_file = fopen(mapfile, "r")) == NULL ) { imklogLogIntMsg(LOG_WARNING, "Cannot open map file."); - dbgprintf("Cannot open map file.\n"); + //dbgprintf("Cannot open map file.\n"); return(0); } } - /* - * Read the kernel symbol table file and add entries for each + /* Read the kernel symbol table file and add entries for each * line. I suspect that the use of fscanf is not really in vogue * but it was quick and dirty and IMHO suitable for fixed format * data such as this. If anybody doesn't agree with this please @@ -217,7 +216,7 @@ extern int InitKsyms(char *mapfile) return(0); } if(dbgPrintSymbols) - dbgprintf("Address: %lx, Type: %c, Symbol: %s\n", address, type, sym); + //dbgprintf("Address: %lx, Type: %c, Symbol: %s\n", address, type, sym); if ( AddSymbol(address, sym) == 0 ) { imklogLogIntMsg(LOG_ERR, "Error adding symbol - %s.", sym); @@ -304,11 +303,11 @@ static char *FindSymbolFile(void) return(0); } - dbgprintf("Searching for symbol map.\n"); + //dbgprintf("Searching for symbol map.\n"); for(mf = system_maps; *mf != NULL && file == NULL; ++mf) { snprintf(mysymfile, sizeof(mysymfile), "%s-%s", *mf, utsname.release); - dbgprintf("Trying %s.\n", mysymfile); + //dbgprintf("Trying %s.\n", mysymfile); if((sym_file = fopen(mysymfile, "r")) != NULL) { if(CheckMapVersion(mysymfile) == 1) file = mysymfile; @@ -316,7 +315,7 @@ static char *FindSymbolFile(void) } if(sym_file == NULL || file == NULL) { sprintf (mysymfile, "%s", *mf); - dbgprintf("Trying %s.\n", mysymfile); + //dbgprintf("Trying %s.\n", mysymfile); if((sym_file = fopen(mysymfile, "r")) != NULL ) { if (CheckMapVersion(mysymfile) == 1) file = mysymfile; @@ -326,7 +325,7 @@ static char *FindSymbolFile(void) } /* At this stage of the game we are at the end of the symbol tables. */ - dbgprintf("End of search list encountered.\n"); + //dbgprintf("End of search list encountered.\n"); ENDfunc return(file); } @@ -397,8 +396,8 @@ static int CheckVersion(char *version) patch = vnum & 0x000000FF; minor = (vnum >> 8) & 0x000000FF; major = (vnum >> 16) & 0x000000FF; - dbgprintf("Version string = %s, Major = %d, Minor = %d, Patch = %d.\n", version + - strlen(prefix), major, minor, patch); + //dbgprintf("Version string = %s, Major = %d, Minor = %d, Patch = %d.\n", version + + // strlen(prefix), major, minor, patch); sprintf(vstring, "%d.%d.%d", major, minor, patch); /* We should now have the version string in the vstring variable in @@ -411,7 +410,7 @@ static int CheckVersion(char *version) imklogLogIntMsg(LOG_ERR, "Cannot get kernel version information."); return(0); } - dbgprintf("Comparing kernel %s with symbol table %s.\n", utsname.release, vstring); + //dbgprintf("Comparing kernel %s with symbol table %s.\n", utsname.release, vstring); if ( sscanf (utsname.release, "%d.%d.%d", &major, &minor, &patch) < 3 ) { imklogLogIntMsg(LOG_ERR, "Kernel send bogus release string `%s'.", utsname.release); @@ -478,7 +477,7 @@ static int CheckMapVersion(char *fname) return(0); } if(dbgPrintSymbols) - dbgprintf("Address: %lx, Type: %c, Symbol: %s\n", address, type, sym); + //dbgprintf("Address: %lx, Type: %c, Symbol: %s\n", address, type, sym); version = CheckVersion(sym); } fclose(sym_file); @@ -488,10 +487,10 @@ static int CheckMapVersion(char *fname) imklogLogIntMsg(LOG_ERR, "Symbol table has incorrect version number.\n"); break; case 0: - dbgprintf("No version information found.\n"); + //dbgprintf("No version information found.\n"); break; case 1: - dbgprintf("Found table with matching version number.\n"); + //dbgprintf("Found table with matching version number.\n"); break; } @@ -791,8 +790,8 @@ extern char *ExpandKadds(char *line, char *el) strcat(elp, symbol); elp += strlen(symbol); - dbgprintf("Symbol: %s = %lx = %s, %x/%d\n", sl+1, value, - (sym.size==0) ? symbol+1 : symbol, sym.offset, sym.size); + //dbgprintf("Symbol: %s = %lx = %s, %x/%d\n", sl+1, value, + // (sym.size==0) ? symbol+1 : symbol, sym.offset, sym.size); value = 2; if ( sym.size != 0 ) { @@ -808,7 +807,7 @@ extern char *ExpandKadds(char *line, char *el) } while ( kp != NULL); - dbgprintf("Expanded line: %s\n", el); + //dbgprintf("Expanded line: %s\n", el); return(el); } diff --git a/plugins/imklog/ksym_mod.c b/plugins/imklog/ksym_mod.c index bef810b4..652effbe 100644 --- a/plugins/imklog/ksym_mod.c +++ b/plugins/imklog/ksym_mod.c @@ -167,7 +167,7 @@ extern int InitMsyms(void) return(0); } - dbgprintf("Loading kernel module symbols - Source: %s\n", KSYMS); + //dbgprintf("Loading kernel module symbols - Source: %s\n", KSYMS); while ( fgets(buf, sizeof(buf), ksyms) != NULL ) { if (num_syms > 0 && index(buf, '[') == NULL) diff --git a/plugins/imklog/linux.c b/plugins/imklog/linux.c index 550ea6a1..31dae2cd 100644 --- a/plugins/imklog/linux.c +++ b/plugins/imklog/linux.c @@ -232,8 +232,8 @@ static void LogLine(char *ptr, int len) */ *line = 0; /* force null terminator */ - dbgprintf("Line buffer full:\n"); - dbgprintf("\tLine: %s\n", line); + //dbgprintf("Line buffer full:\n"); + //dbgprintf("\tLine: %s\n", line); Syslog(LOG_INFO, line_buff); line = line_buff; @@ -504,7 +504,7 @@ rsRetVal klogWillRun(void) symbol_lookup = (InitKsyms(symfile) == 1); symbol_lookup |= InitMsyms(); if (symbol_lookup == 0) { - dbgprintf("cannot find any symbols, turning off symbol lookups\n"); + //dbgprintf("cannot find any symbols, turning off symbol lookups\n"); imklogLogIntMsg(LOG_WARNING, "cannot find any symbols, turning off symbol lookups"); } } -- cgit From dc423197ba473a94748e571a48e3130b58f4850f Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Wed, 30 Apr 2008 18:59:01 +0200 Subject: bugfix: segfault in imklog A symbol file was closed when it couldn't opened. That lead to a NULL pointer being passed to fclose() --- plugins/imklog/ksym_mod.c | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/imklog/ksym_mod.c b/plugins/imklog/ksym_mod.c index 652effbe..dae0e13d 100644 --- a/plugins/imklog/ksym_mod.c +++ b/plugins/imklog/ksym_mod.c @@ -163,7 +163,6 @@ extern int InitMsyms(void) else imklogLogIntMsg(LOG_ERR, "Error loading kernel symbols " \ "- %s\n", strerror(errno)); - fclose(ksyms); return(0); } -- cgit