2000-12-06 22:56:02

by Georg Nikodym

[permalink] [raw]
Subject: linux-2.4.0-test11 and sysklogd-1.3-31


sysklogd 1.3-31 no longer compiles using the latest headers in test11.

Strictly speaking this isn't a kernel bug...

sysklogd's ksym_mod.c includes <linux/module.h>

In test11, <linux/module.h> added struct inter_module_entry. Its
first member is "struct list_head list;". This necessitates the
inclusion of <linux/list.h>.

The trouble is that <linux/list.h> is almost completely protected by
#ifdef __KERNEL__.

sysklogd, obviously, doesn't compile with __KERNEL__ so the struct
inter_module_entry declaration is impossible and the compilation
fails.

It's not clear to me who's code needs changing so I'm sending both to
linux-kernel and to some of the people that have the misfortune of
being listed on the sysklogd man page.


2000-12-07 00:18:10

by Keith Owens

[permalink] [raw]
Subject: Re: linux-2.4.0-test11 and sysklogd-1.3-31

On Wed, 6 Dec 2000 17:24:58 -0500 (EST),
"Georg Nikodym" <[email protected]> wrote:
>sysklogd 1.3-31 no longer compiles using the latest headers in test11.
>
>Strictly speaking this isn't a kernel bug...
>
>sysklogd's ksym_mod.c includes <linux/module.h>

Speaking as the modutils maintainer and the person who added list.h to
module.h, sysklogd should *not* be including linux/module.h. Linus has
spoken, it is an error for user space applications to include kernel
headers. Even modutils does not include linux/module.h, instead it has
a portable (2.0 through 2.4) local definition of struct module.

ksym_mod.c is only present to try to decode oops reports in klogd.
klogd only handles some architectures, it often gets the oops decode
wrong and it destroys the log information that is needed by other oops
decoders. IMNSHO ksymoops does a much better job of decoding the oops,
but I maintain ksymoops so I am just a little biased ;)

I would prefer to see the oops decoding completely removed from klogd.
The only justification for klogd converting the oops is to save users
from running ksymoops by hand. I would not mind klogd capturing the
oops text, forking to run ksymoops then logging the ksymoops output.
Just as along as the original text was left alone and the ksymoops
output was obviously distinguished from real kernel output.

2000-12-07 18:07:27

by Georg Nikodym

[permalink] [raw]
Subject: Re: linux-2.4.0-test11 and sysklogd-1.3-31

>>>>> "KO" == Keith Owens <[email protected]> writes:

KO> I would prefer to see the oops decoding completely removed from
KO> klogd. The only justification for klogd converting the oops is
KO> to save users from running ksymoops by hand. I would not mind
KO> klogd capturing the oops text, forking to run ksymoops then
KO> logging the ksymoops output. Just as along as the original text
KO> was left alone and the ksymoops output was obviously
KO> distinguished from real kernel output.

Since nobody else has weighed in on this issue, I quickly did the
necessary to effect Keith's suggestion. What follows is a patch to
sysklogd-1.3-31 (which after applying, ksym_mod.c can be removed):

# This is a BitKeeper generated patch for the following project:
# Project Name: Trans-lab fsimage sub-gate
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
# ChangeSet 1.60 -> 1.62
# src/sysklogd-1.3-31/ksym.c 1.1 -> 1.2
# src/sysklogd-1.3-31/klogd.c 1.1 -> 1.2
# src/sysklogd-1.3-31/ksyms.h 1.1 -> 1.2
# src/sysklogd-1.3-31/Makefile 1.1 -> 1.2
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 00/12/07 [email protected] 1.61
# Remove ksym_mod.c to fix sysklogd build
# --------------------------------------------
# 00/12/07 [email protected] 1.62
# Remove a remaining prototype associated with the now deleted ksym_mod.c
# --------------------------------------------
#
diff -Nru a/src/sysklogd-1.3-31/Makefile b/src/sysklogd-1.3-31/Makefile
--- a/src/sysklogd-1.3-31/Makefile Thu Dec 7 11:53:56 2000
+++ b/src/sysklogd-1.3-31/Makefile Thu Dec 7 11:53:56 2000
@@ -63,9 +63,8 @@
syslogd: syslogd.o pidfile.o
${CC} ${LDFLAGS} -o syslogd syslogd.o pidfile.o ${LIBS}

-klogd: klogd.o syslog.o pidfile.o ksym.o ksym_mod.o
- ${CC} ${LDFLAGS} -o klogd klogd.o syslog.o pidfile.o ksym.o \
- ksym_mod.o ${LIBS}
+klogd: klogd.o syslog.o pidfile.o ksym.o
+ ${CC} ${LDFLAGS} -o klogd klogd.o syslog.o pidfile.o ksym.o ${LIBS}

syslog_tst: syslog_tst.o
${CC} ${LDFLAGS} -o syslog_tst syslog_tst.o
diff -Nru a/src/sysklogd-1.3-31/klogd.c b/src/sysklogd-1.3-31/klogd.c
--- a/src/sysklogd-1.3-31/klogd.c Thu Dec 7 11:53:56 2000
+++ b/src/sysklogd-1.3-31/klogd.c Thu Dec 7 11:53:56 2000
@@ -415,7 +415,6 @@
if (symbol_lookup) {
if ( reload_symbols > 1 )
InitKsyms(symfile);
- InitMsyms();
}
reload_symbols = change_state = 0;
return;
@@ -1059,7 +1058,6 @@
{
if (symbol_lookup) {
InitKsyms(symfile);
- InitMsyms();
}
if ( (logsrc = GetKernelLogSrc()) == kernel )
LogKernelLine();
@@ -1075,7 +1073,6 @@
logsrc = GetKernelLogSrc();
if (symbol_lookup) {
InitKsyms(symfile);
- InitMsyms();
}

/* The main loop. */
diff -Nru a/src/sysklogd-1.3-31/ksym.c b/src/sysklogd-1.3-31/ksym.c
--- a/src/sysklogd-1.3-31/ksym.c Thu Dec 7 11:53:56 2000
+++ b/src/sysklogd-1.3-31/ksym.c Thu Dec 7 11:53:56 2000
@@ -656,9 +656,6 @@
last = sym_array[lp].name;
}

- if ( (last = LookupModuleSymbol(value, sym)) != (char *) 0 )
- return(last);
-
return((char *) 0);
}

@@ -749,7 +746,7 @@
* open for patches.
*/
if ( i_am_paranoid &&
- (strstr(line, "Oops:") != (char *) 0) && !InitMsyms() )
+ (strstr(line, "Oops:") != (char *) 0) )
Syslog(LOG_WARNING, "Cannot load kernel module symbols.\n");


diff -Nru a/src/sysklogd-1.3-31/ksyms.h b/src/sysklogd-1.3-31/ksyms.h
--- a/src/sysklogd-1.3-31/ksyms.h Thu Dec 7 11:53:56 2000
+++ b/src/sysklogd-1.3-31/ksyms.h Thu Dec 7 11:53:56 2000
@@ -32,4 +32,3 @@

/* Function prototypes. */
extern char * LookupSymbol(unsigned long, struct symbol *);
-extern char * LookupModuleSymbol(unsigned long int, struct symbol *);

2000-12-07 23:22:44

by Keith Owens

[permalink] [raw]
Subject: Re: linux-2.4.0-test11 and sysklogd-1.3-31

On Thu, 7 Dec 2000 12:36:01 -0500 (EST),
"Georg Nikodym" <[email protected]> wrote:
>>>>>> "KO" == Keith Owens <[email protected]> writes:
>
> KO> I would prefer to see the oops decoding completely removed from
> KO> klogd.
>
>Since nobody else has weighed in on this issue, I quickly did the
>necessary to effect Keith's suggestion. What follows is a patch to
>sysklogd-1.3-31 (which after applying, ksym_mod.c can be removed):

You only removed the module symbol handling. The problem is that the
entire klogd oops handling is out of date and broken. I recommend
removing all oops processing from klogd, which means that klogd does
not need any symbols nor System.map.

2000-12-08 17:00:52

by Georg Nikodym

[permalink] [raw]
Subject: Re: linux-2.4.0-test11 and sysklogd-1.3-31

>>>>> "KO" == Keith Owens <[email protected]> writes:

KO> You only removed the module symbol handling. The problem is that
KO> the entire klogd oops handling is out of date and broken. I
KO> recommend removing all oops processing from klogd, which means
KO> that klogd does not need any symbols nor System.map.

You're right.

My goal was to quickly get our build working again. I had no
expectation that anybody else on the planet would give a crap about my
patch...

But since you seem to and while we're doing extreme surgery, why have
klogd at all? Every other unix, kernel messages are handled by the
syslog system. What problem did klogd solve and does that problem
still exist today? Stated differently, aren't you just proposing to
reduce klogd to:

cat < /proc/kmsg > /dev/log &

(I know that this won't work on my box, but you get the idea)

Anyway, I'll look into simplifying klogd as you suggest...

2000-12-08 22:46:59

by Keith Owens

[permalink] [raw]
Subject: Re: linux-2.4.0-test11 and sysklogd-1.3-31

On Fri, 8 Dec 2000 11:30:06 -0500 (EST),
"Georg Nikodym" <[email protected]> wrote:
>But since you seem to and while we're doing extreme surgery, why have
>klogd at all? Every other unix, kernel messages are handled by the
>syslog system. What problem did klogd solve and does that problem
>still exist today?

klogd maps the kernel messages from <n>text to syslog levels and does
some fiddling with kernel log levels at start up. It needs to be more
than a simple 'cat'. When symbol handling was added to klogd, ksymoops
was built into the kernel and very unreliable. Since then ksymoops has
been moved to a separate package and is now reliable. Alas oops
handling in sysklogd has not kept up to date and is now the problem
area.

2000-12-12 01:44:44

by Georg Nikodym

[permalink] [raw]
Subject: Re: linux-2.4.0-test11 and sysklogd-1.3-31

>>>>> "KO" == Keith Owens <[email protected]> writes:

KO> klogd maps the kernel messages from <n>text to syslog levels and
KO> does some fiddling with kernel log levels at start up. It needs
KO> to be more than a simple 'cat'. When symbol handling was added
KO> to klogd, ksymoops was built into the kernel and very unreliable.
KO> Since then ksymoops has been moved to a separate package and is
KO> now reliable. Alas oops handling in sysklogd has not kept up to
KO> date and is now the problem area.

Thanks for the background.

KO> Linus, please do not apply.

KO> User space applications _must_ not include kernel headers. Even
KO> modutils does not include linux/module.h, it has its own portable
KO> (kernels 2.0 - 2.4) version.

KO> I have strongly recommended to the sysklogd maintainers that they
KO> strip all the symbol handling from klogd. The oops decoding in
KO> klogd is hopelessly out of date and broken.

Here's a patch (against sysklogd-1.3-31) that completely tear out the
symbol processing code.

Additionally, after application, the following files may be removed:

ksyms.h
ksym.c
ksym_mod.c


diff -Nru a/src/sysklogd-1.3-31/Makefile b/src/sysklogd-1.3-31/Makefile
--- a/src/sysklogd-1.3-31/Makefile Mon Dec 11 19:32:22 2000
+++ b/src/sysklogd-1.3-31/Makefile Mon Dec 11 19:32:22 2000
@@ -63,9 +63,8 @@
syslogd: syslogd.o pidfile.o
${CC} ${LDFLAGS} -o syslogd syslogd.o pidfile.o ${LIBS}

-klogd: klogd.o syslog.o pidfile.o ksym.o ksym_mod.o
- ${CC} ${LDFLAGS} -o klogd klogd.o syslog.o pidfile.o ksym.o \
- ksym_mod.o ${LIBS}
+klogd: klogd.o syslog.o pidfile.o ksym.o
+ ${CC} ${LDFLAGS} -o klogd klogd.o syslog.o pidfile.o ksym.o ${LIBS}

syslog_tst: syslog_tst.o
${CC} ${LDFLAGS} -o syslog_tst syslog_tst.o
diff -Nru a/src/sysklogd-1.3-31/klogd.c b/src/sysklogd-1.3-31/klogd.c
--- a/src/sysklogd-1.3-31/klogd.c Mon Dec 11 19:32:22 2000
+++ b/src/sysklogd-1.3-31/klogd.c Mon Dec 11 19:32:22 2000
@@ -415,7 +415,6 @@
if (symbol_lookup) {
if ( reload_symbols > 1 )
InitKsyms(symfile);
- InitMsyms();
}
reload_symbols = change_state = 0;
return;
@@ -1059,7 +1058,6 @@
{
if (symbol_lookup) {
InitKsyms(symfile);
- InitMsyms();
}
if ( (logsrc = GetKernelLogSrc()) == kernel )
LogKernelLine();
@@ -1075,7 +1073,6 @@
logsrc = GetKernelLogSrc();
if (symbol_lookup) {
InitKsyms(symfile);
- InitMsyms();
}

/* The main loop. */
diff -Nru a/src/sysklogd-1.3-31/ksym.c b/src/sysklogd-1.3-31/ksym.c
--- a/src/sysklogd-1.3-31/ksym.c Mon Dec 11 19:32:22 2000
+++ b/src/sysklogd-1.3-31/ksym.c Mon Dec 11 19:32:22 2000
@@ -656,9 +656,6 @@
last = sym_array[lp].name;
}

- if ( (last = LookupModuleSymbol(value, sym)) != (char *) 0 )
- return(last);
-
return((char *) 0);
}

@@ -749,7 +746,7 @@
* open for patches.
*/
if ( i_am_paranoid &&
- (strstr(line, "Oops:") != (char *) 0) && !InitMsyms() )
+ (strstr(line, "Oops:") != (char *) 0) )
Syslog(LOG_WARNING, "Cannot load kernel module symbols.\n");


diff -Nru a/src/sysklogd-1.3-31/ksyms.h b/src/sysklogd-1.3-31/ksyms.h
--- a/src/sysklogd-1.3-31/ksyms.h Mon Dec 11 19:32:23 2000
+++ b/src/sysklogd-1.3-31/ksyms.h Mon Dec 11 19:32:23 2000
@@ -32,4 +32,3 @@

/* Function prototypes. */
extern char * LookupSymbol(unsigned long, struct symbol *);
-extern char * LookupModuleSymbol(unsigned long int, struct symbol *);
diff -Nru a/src/sysklogd-1.3-31/klogd.8 b/src/sysklogd-1.3-31/klogd.8
--- a/src/sysklogd-1.3-31/klogd.8 Mon Dec 11 19:32:23 2000
+++ b/src/sysklogd-1.3-31/klogd.8 Mon Dec 11 19:32:23 2000
@@ -3,6 +3,8 @@
.\" Sun Jul 30 01:35:55 MET: Martin Schulze: Updates
.\" Sun Nov 19 23:22:21 MET: Martin Schulze: Updates
.\" Mon Aug 19 09:42:08 CDT 1996: Dr. G.W. Wettstein: Updates
+.\" Mon Dec 11 2000: Georg Nikodym: Removal of documentation related to
+.\" symbol resolution (the code has been removed).
.\"
.TH KLOGD 8 "24 November 1995" "Version 1.3" "Linux System Administration"
.SH NAME
@@ -17,16 +19,10 @@
.RB [ " \-f "
.I fname
]
-.RB [ " \-iI " ]
.RB [ " \-n " ]
.RB [ " \-o " ]
-.RB [ " \-p " ]
.RB [ " \-s " ]
-.RB [ " \-k "
-.I fname
-]
.RB [ " \-v " ]
-.RB [ " \-x " ]
.LP
.SH DESCRIPTION
.B klogd
@@ -45,12 +41,6 @@
.BI "\-f " file
Log messages to the specified filename rather than to the syslog facility.
.TP
-.BI "\-i \-I"
-Signal the currently executing klogd daemon. Both of these switches control
-the loading/reloading of symbol information. The \-i switch signals the
-daemon to reload the kernel module symbols. The \-I switch signals for a
-reload of both the static kernel symbols and the kernel module symbols.
-.TP
.B "\-n"
Avoid auto-backgrounding. This is needed especially if the
.B klogd
@@ -62,24 +52,12 @@
all the messages that are found in the kernel message buffers. After
a single read and log cycle the daemon exits.
.TP
-.B "-p"
-Enable paranoia. This option controls when klogd loads kernel module symbol
-information. Setting this switch causes klogd to load the kernel module
-symbol information whenever an Oops string is detected in the kernel message
-stream.
-.TP
.B "-s"
Force \fBklogd\fP to use the system call interface to the kernel message
buffers.
.TP
-.BI "\-k " file
-Use the specified file as the source of kernel symbol information.
-.TP
.B "\-v"
Print version and exit.
-.TP
-.B "\-x"
-Omits EIP translation and there doesn't read the System.map.
.LP
.SH OVERVIEW
The functionality of klogd has been typically incorporated into other
@@ -214,37 +192,6 @@
.B ksymoops
program which is included in the kernel sources.

-As a convenience
-.B klogd
-will attempt to resolve kernel numeric addresses to their symbolic
-forms if a kernel symbol table is available at execution time. A
-symbol table may be specified by using the \fB\-k\fR switch on the
-command line. If a symbol file is not explicitly specified the
-following filenames will be tried:
-
-.nf
-.I /boot/System.map
-.I /System.map
-.I /usr/src/linux/System.map
-.fi
-
-Version information is supplied in the system maps as of kernel
-1.3.43. This version information is used to direct an intelligent
-search of the list of symbol tables. This feature is useful since it
-provides support for both production and experimental kernels.
-
-For example a production kernel may have its map file stored in
-/boot/System.map. If an experimental or test kernel is compiled with
-the sources in the 'standard' location of /usr/src/linux the system
-map will be found in /usr/src/linux/System.map. When klogd starts
-under the experimental kernel the map in /boot/System.map will be
-bypassed in favor of the map in /usr/src/linux/System.map.
-
-Modern kernels as of 1.3.43 properly format important kernel addresses
-so that they will be recognized and translated by klogd. Earlier
-kernels require a source code patch be applied to the kernel sources.
-This patch is supplied with the sysklogd sources.
-
The process of analyzing kernel protections faults works very well
with a static kernel. Additional difficulties are encountered when
attempting to diagnose errors which occur in loadable kernel modules.
@@ -262,70 +209,6 @@
loadable module. Without this location map it is not possible for a
kernel developer to determine what went wrong if a protection fault
involves a kernel module.
-
-.B klogd
-has support for dealing with the problem of diagnosing protection
-faults in kernel loadable modules. At program start time or in
-response to a signal the daemon will interrogate the kernel for a
-listing of all modules loaded and the addresses in memory they are
-loaded at. Individual modules can also register the locations of
-important functions when the module is loaded. The addresses of these
-exported symbols are also determined during this interrogation
-process.
-
-When a protection fault occurs an attempt will be made to resolve
-kernel addresses from the static symbol table. If this fails the
-symbols from the currently loaded modules are examined in an attempt
-to resolve the addresses. At the very minimum this allows klogd to
-indicate which loadable module was responsible for generating the
-protection fault. Additional information may be available if the
-module developer chose to export symbol information from the module.
-
-Proper and accurate resolution of addresses in kernel modules requires
-that
-.B klogd
-be informed whenever the kernel module status changes. The
-.B \-i
-and
-.B \-I
-switches can be used to signal the currently executing daemon that
-symbol information be reloaded. Of most importance to proper
-resolution of module symbols is the
-.B \-i
-switch. Each time a kernel module is loaded or removed from the
-kernel the following command should be executed:
-
-.nf
-.I klogd \-i
-.fi
-
-The
-.B \-p
-switch can also be used to insure that module symbol information is up
-to date. This switch instructs
-.B klogd
-to reload the module symbol information whenever a protection fault
-is detected. Caution should be used before invoking the program in
-\'paranoid\' mode. The stability of the kernel and the operating
-environment is always under question when a protection fault occurs.
-Since the klogd daemon must execute system calls in order to read the
-module symbol information there is the possibility that the system may
-be too unstable to capture useful information. A much better policy
-is to insure that klogd is updated whenever a module is loaded or
-unloaded. Having uptodate symbol information loaded increases the
-probability of properly resolving a protection fault if it should occur.
-
-Included in the sysklogd source distribution is a patch to the
-modules-2.0.0 package which allows the
-.B insmod,
-.B rmmod
-and
-.B modprobe
-utilities to automatically signal
-.B klogd
-whenever a module is inserted or removed from the kernel. Using this
-patch will insure that the symbol information maintained in klogd is
-always consistent with the current kernel state.
.PP
.SH SIGNAL HANDLING
The
@@ -363,26 +246,6 @@
.B LOG_INFO
priority
documenting the start/stop of logging.
-
-The
-.BR SIGUSR1 " and " SIGUSR2
-signals are used to initiate loading/reloading of kernel symbol information.
-Receipt of the
-.B SIGUSR1
-signal will cause the kernel module symbols to be reloaded. Signaling the
-daemon with
-.B SIGUSR2
-will cause both the static kernel symbols and the kernel module symbols to
-be reloaded.
-
-Provided that the System.map file is placed in an appropriate location the
-signal of generally greatest usefulness is the
-.B SIGUSR1
-signal. This signal is designed to be used to signal the daemon when kernel
-modules are loaded/unloaded. Sending this signal to the daemon after a
-kernel module state change will insure that proper resolution of symbols will
-occur if a protection fault occurs in the address space occupied by a kernel
-module.
.LP
.SH FILES
.PD 0
diff -Nru a/src/sysklogd-1.3-31/klogd.c b/src/sysklogd-1.3-31/klogd.c
--- a/src/sysklogd-1.3-31/klogd.c Mon Dec 11 19:32:23 2000
+++ b/src/sysklogd-1.3-31/klogd.c Mon Dec 11 19:32:23 2000
@@ -213,6 +213,10 @@
* Shortened LOG_LINE_LENGTH in order to get long lines splitted
* up earlier and syslogd has a better chance concatenating them
* together again.
+ *
+ * Mon Dec 11, 2000: Georg Nikodym <[email protected]>
+ * Removed all support for symbol resolution at the suggestion of
+ * Keith Owens.
*/


@@ -229,7 +233,6 @@
#include <paths.h>
#include <stdlib.h>
#include "klogd.h"
-#include "ksyms.h"
#ifndef TESTING
#include "pidfile.h"
#endif
@@ -260,16 +263,13 @@
change_state = 0,
terminate = 0,
caught_TSTP = 0,
- reload_symbols = 0,
console_log_level = 6;

static int use_syscall = 0,
one_shot = 0,
- symbol_lookup = 1,
no_fork = 0; /* don't fork - don't run in daemon mode */

-static char *symfile = (char *) 0,
- log_buffer[LOG_BUFFER_SIZE];
+static char log_buffer[LOG_BUFFER_SIZE];

static FILE *output_file = (FILE *) 0;

@@ -287,7 +287,6 @@
extern void reload_daemon(int sig);
static void Terminate(void);
static void SignalDaemon(int);
-static void ReloadSymbols(void);
static void ChangeLogging(void);
static enum LOGSRC GetKernelLogSrc(void);
static void LogLine(char *ptr, int len);
@@ -363,12 +362,10 @@

{
change_state = 1;
- reload_symbols = 1;


if ( sig == SIGUSR2 )
{
- ++reload_symbols;
signal(SIGUSR2, reload_daemon);
}
else
@@ -409,17 +406,6 @@
}


-static void ReloadSymbols()
-
-{
- if (symbol_lookup) {
- if ( reload_symbols > 1 )
- InitKsyms(symfile);
- }
- reload_symbols = change_state = 0;
- return;
-}
-

static void ChangeLogging(void)

@@ -432,13 +418,6 @@
Syslog(LOG_INFO, "klogd %s-%s, ---------- state change ----------\n", \
VERSION, PATCHLEVEL);

- /* Reload symbols. */
- if ( reload_symbols > 0 )
- {
- ReloadSymbols();
- return;
- }
-
/* Stop kernel logging. */
if ( caught_TSTP == 1 )
{
@@ -627,24 +606,11 @@
/*
* Messages are separated by "\n". Messages longer than
* LOG_LINE_LENGTH are broken up.
- *
- * Kernel symbols show up in the input buffer as : "[<aaaaaa>]",
- * where "aaaaaa" is the address. These are replaced with
- * "[symbolname+offset/size]" in the output line - symbolname,
- * offset, and size come from the kernel symbol table.
- *
- * If a kernel symbol happens to fall at the end of a message close
- * in length to LOG_LINE_LENGTH, the symbol will not be expanded.
- * (This should never happen, since the kernel should never generate
- * messages that long.
*/
static void LogLine(char *ptr, int len)
{
enum parse_state_enum {
- PARSING_TEXT,
- PARSING_SYMSTART, /* at < */
- PARSING_SYMBOL,
- PARSING_SYMEND /* at ] */
+ PARSING_TEXT
};

static char line_buff[LOG_LINE_LENGTH];
@@ -653,8 +619,6 @@
static enum parse_state_enum parse_state = PARSING_TEXT;
static int space = sizeof(line_buff)-1;

- static char *sym_start; /* points at the '<' of a symbol */
-
auto int delta = 0; /* number of chars copied */

while( len > 0 )
@@ -678,165 +642,53 @@
parse_state = PARSING_TEXT;
}

- switch( parse_state )
- {
- case PARSING_TEXT:
- delta = copyin( line, space, ptr, len, "\n[%" );
- line += delta;
- ptr += delta;
- space -= delta;
- len -= delta;
-
- if( space == 0 || len == 0 )
- {
- break; /* full line_buff or end of input buffer */
- }
-
- if( *ptr == '\n' ) /* newline */
- {
- *line++ = *ptr++; /* copy it in */
- space -= 1;
- len -= 1;
-
- *line = 0; /* force null terminator */
- Syslog( LOG_INFO, line_buff );
- line = line_buff;
- space = sizeof(line_buff)-1;
- break;
- }
- if( *ptr == '[' ) /* possible kernel symbol */
- {
- *line++ = *ptr++;
- space -= 1;
- len -= 1;
- parse_state = PARSING_SYMSTART; /* at < */
- break;
- }
- if( *ptr == '%' ) /* dangerous printf marker */
- {
- delta = 0;
- while (len && *ptr == '%')
- {
- *line++ = *ptr++; /* copy it in */
- space -= 1;
- len -= 1;
- delta++;
- }
- if (delta % 2) /* odd amount of %'s */
- {
- if (space)
- {
- *line++ = '%'; /* so simply add one */
- space -= 1;
- }
- else
- {
- *line++ = '\0'; /* remove the last one / terminate the string */
- }
-
- }
- }
- break;
-
- case PARSING_SYMSTART:
- if( *ptr != '<' )
- {
- parse_state = PARSING_TEXT; /* not a symbol */
- break;
- }
-
- /*
- ** Save this character for now. If this turns out to
- ** be a valid symbol, this char will be replaced later.
- ** If not, we'll just leave it there.
- */
-
- sym_start = line; /* this will point at the '<' */
-
- *line++ = *ptr++;
- space -= 1;
- len -= 1;
- parse_state = PARSING_SYMBOL; /* symbol... */
- break;
-
- case PARSING_SYMBOL:
- delta = copyin( line, space, ptr, len, ">\n[" );
- line += delta;
- ptr += delta;
- space -= delta;
- len -= delta;
- if( space == 0 || len == 0 )
- {
- break; /* full line_buff or end of input buffer */
- }
- if( *ptr != '>' )
- {
- parse_state = PARSING_TEXT;
- break;
- }
-
- *line++ = *ptr++; /* copy the '>' */
- space -= 1;
- len -= 1;
-
- parse_state = PARSING_SYMEND;
-
- break;
-
- case PARSING_SYMEND:
- if( *ptr != ']' )
- {
- parse_state = PARSING_TEXT; /* not a symbol */
- break;
- }
-
- /*
- ** It's really a symbol! Replace address with the
- ** symbol text.
- */
- {
- auto int sym_space;
-
- unsigned long value;
- auto struct symbol sym;
- auto char *symbol;
-
- *(line-1) = 0; /* null terminate the address string */
- value = strtoul(sym_start+1, (char **) 0, 16);
- *(line-1) = '>'; /* put back delim */
-
- symbol = LookupSymbol(value, &sym);
- if ( !symbol_lookup || symbol == (char *) 0 )
- {
- parse_state = PARSING_TEXT;
- break;
- }
-
- /*
- ** verify there is room in the line buffer
- */
- sym_space = space + ( line - sym_start );
- if( sym_space < strlen(symbol) + 30 ) /*(30 should be overkill)*/
- {
- parse_state = PARSING_TEXT; /* not enough space */
- break;
- }
-
- delta = sprintf( sym_start, "%s+%d/%d]",
- symbol, sym.offset, sym.size );
-
- space = sym_space + delta;
- line = sym_start + delta;
- }
- ptr++;
- len--;
- parse_state = PARSING_TEXT;
- break;
-
- default: /* Can't get here! */
- parse_state = PARSING_TEXT;
+ delta = copyin( line, space, ptr, len, "\n%" );
+ line += delta;
+ ptr += delta;
+ space -= delta;
+ len -= delta;
+
+ if( space == 0 || len == 0 )
+ {
+ break; /* full line_buff or end of input buffer */
+ }
+
+ if( *ptr == '\n' ) /* newline */
+ {
+ *line++ = *ptr++; /* copy it in */
+ space -= 1;
+ len -= 1;
+
+ *line = 0; /* force null terminator */
+ Syslog( LOG_INFO, line_buff );
+ line = line_buff;
+ space = sizeof(line_buff)-1;
+ break;
+ }
+ if( *ptr == '%' ) /* dangerous printf marker */
+ {
+ delta = 0;
+ while (len && *ptr == '%')
+ {
+ *line++ = *ptr++; /* copy it in */
+ space -= 1;
+ len -= 1;
+ delta++;
+ }
+ if (delta % 2) /* odd amount of %'s */
+ {
+ if (space)
+ {
+ *line++ = '%'; /* so simply add one */
+ space -= 1;
+ }
+ else
+ {
+ *line++ = '\0'; /* remove the last one / terminate the string */
+ }

- }
+ }
+ }
}

return;
@@ -911,7 +763,7 @@
chdir ("/");
#endif
/* Parse the command-line. */
- while ((ch = getopt(argc, argv, "c:df:iIk:nopsvx")) != EOF)
+ while ((ch = getopt(argc, argv, "c:df:nosv")) != EOF)
switch((char)ch)
{
case 'c': /* Set console message level. */
@@ -924,33 +776,18 @@
output = optarg;
use_output++;
break;
- case 'i': /* Reload module symbols. */
- SignalDaemon(SIGUSR1);
- return(0);
- case 'I':
- SignalDaemon(SIGUSR2);
- return(0);
- case 'k': /* Kernel symbol file. */
- symfile = optarg;
- break;
case 'n': /* don't fork */
no_fork++;
break;
case 'o': /* One-shot mode. */
one_shot = 1;
break;
- case 'p':
- SetParanoiaLevel(1); /* Load symbols on oops. */
- break;
case 's': /* Use syscall interface. */
use_syscall = 1;
break;
case 'v':
printf("klogd %s-%s\n", VERSION, PATCHLEVEL);
exit (1);
- case 'x':
- symbol_lookup = 0;
- break;
}


@@ -1056,9 +893,6 @@
/* Handle one-shot logging. */
if ( one_shot )
{
- if (symbol_lookup) {
- InitKsyms(symfile);
- }
if ( (logsrc = GetKernelLogSrc()) == kernel )
LogKernelLine();
else
@@ -1071,9 +905,6 @@
sleep(KLOGD_DELAY);
#endif
logsrc = GetKernelLogSrc();
- if (symbol_lookup) {
- InitKsyms(symfile);
- }

/* The main loop. */
while (1)
diff -Nru a/src/sysklogd-1.3-31/klogd.h b/src/sysklogd-1.3-31/klogd.h
--- a/src/sysklogd-1.3-31/klogd.h Mon Dec 11 19:32:23 2000
+++ b/src/sysklogd-1.3-31/klogd.h Mon Dec 11 19:32:23 2000
@@ -33,8 +33,4 @@


/* Function prototypes. */
-extern int InitKsyms(char *);
-extern int InitMsyms(void);
-extern char * ExpandKadds(char *, char *);
-extern void SetParanoiaLevel(int);
extern void Syslog(int priority, char *fmt, ...);

2000-12-12 02:00:17

by Georg Nikodym

[permalink] [raw]
Subject: Re: linux-2.4.0-test11 and sysklogd-1.3-31

>>>>> "GN" == Georg Nikodym <[email protected]> writes:

GN> Here's a patch (against sysklogd-1.3-31) that completely tear out
GN> the symbol processing code.

Doh! Forgot a chunk (to be applied after the others):

diff -Nru a/src/sysklogd-1.3-31/Makefile b/src/sysklogd-1.3-31/Makefile
--- a/src/sysklogd-1.3-31/Makefile Mon Dec 11 20:28:24 2000
+++ b/src/sysklogd-1.3-31/Makefile Mon Dec 11 20:28:24 2000
@@ -63,8 +63,8 @@
syslogd: syslogd.o pidfile.o
${CC} ${LDFLAGS} -o syslogd syslogd.o pidfile.o ${LIBS}

-klogd: klogd.o syslog.o pidfile.o ksym.o
- ${CC} ${LDFLAGS} -o klogd klogd.o syslog.o pidfile.o ksym.o ${LIBS}
+klogd: klogd.o syslog.o pidfile.o
+ ${CC} ${LDFLAGS} -o klogd klogd.o syslog.o pidfile.o ${LIBS}

syslog_tst: syslog_tst.o
${CC} ${LDFLAGS} -o syslog_tst syslog_tst.o

2000-12-12 02:01:26

by Keith Owens

[permalink] [raw]
Subject: Re: linux-2.4.0-test11 and sysklogd-1.3-31

On Mon, 11 Dec 2000 20:13:46 -0500 (EST),
"Georg Nikodym" <[email protected]> wrote:
>Here's a patch (against sysklogd-1.3-31) that completely tear out the
>symbol processing code.

Looks good, except that you need to keep the option flags for backwards
compatibility. There are a *lot* of scripts out there which invoke
klogd with various options and they will fail with this change. It is
OK to issue a warning message "klogd options -[iIpkx] are no longer
supported" as long as klogd continues to run. Otherwise you will get a
lot of irate users complaining that klogd is failing at boot time.

2000-12-12 02:24:16

by Georg Nikodym

[permalink] [raw]
Subject: Re: linux-2.4.0-test11 and sysklogd-1.3-31

>>>>> "KO" == Keith Owens <[email protected]> writes:

KO> Looks good, except that you need to keep the option flags for
KO> backwards compatibility. There are a *lot* of scripts out there
KO> which invoke klogd with various options and they will fail with
KO> this change. It is OK to issue a warning message "klogd options
KO> -[iIpkx] are no longer supported" as long as klogd continues to
KO> run. Otherwise you will get a lot of irate users complaining
KO> that klogd is failing at boot time.

You're right. Here's YAP:

diff -Nru a/src/sysklogd-1.3-31/klogd.c b/src/sysklogd-1.3-31/klogd.c
--- a/src/sysklogd-1.3-31/klogd.c Mon Dec 11 20:50:49 2000
+++ b/src/sysklogd-1.3-31/klogd.c Mon Dec 11 20:50:49 2000
@@ -763,7 +763,7 @@
chdir ("/");
#endif
/* Parse the command-line. */
- while ((ch = getopt(argc, argv, "c:df:nosv")) != EOF)
+ while ((ch = getopt(argc, argv, "c:df:nosviIk:px")) != EOF)
switch((char)ch)
{
case 'c': /* Set console message level. */
@@ -788,6 +788,20 @@
case 'v':
printf("klogd %s-%s\n", VERSION, PATCHLEVEL);
exit (1);
+
+ /* Obsolete options */
+ case 'i':
+ /* FALLTHRU */
+ case 'I':
+ /* FALLTHRU */
+ case 'k':
+ /* FALLTHRU */
+ case 'p':
+ /* FALLTHRU */
+ case 'x':
+ fprintf(stderr,
+ "klogd: %c option is obsolete. Ignoring\n", ch);
+ break;
}


2000-12-12 04:28:34

by Peter Samuelson

[permalink] [raw]
Subject: Re: linux-2.4.0-test11 and sysklogd-1.3-31


[Georg Nikodym]
> + case 'x':
> + fprintf(stderr,
> + "klogd: %c option is obsolete. Ignoring\n", ch);

Clearer (IMHO): "klogd: warning: ignoring obsolete option '-%c'\n", ch);

Peter