Jakub Filak jfilak@redhat.com writes:
- closes rhbz#892230
Signed-off-by: Jakub Filak jfilak@redhat.com
src/lib/kernel.c | 145 ++++++++++++++++++++++++++++--------------------------- 1 file changed, 74 insertions(+), 71 deletions(-)
ok, this definitely needs to be as two separate patches. first, and it could be vice versa, could be to moving suspicious string out of koops_print_suspicious_strings to s_suspicious_strings. Why did you use *s_* prefix in s_suspicious_strings anyway? Correct name should be koops_suspicious_string_arr. And the second could be adding new strings to array of suspicious strings.
diff --git a/src/lib/kernel.c b/src/lib/kernel.c index 6b13362..32d05a0 100644 --- a/src/lib/kernel.c +++ b/src/lib/kernel.c @@ -74,25 +74,58 @@ static void record_oops(GList **oops_list, struct line_info* lines_info, int oop VERB3 if (rv == 0) log("Dropped oops: too short"); }
+/* In some comparisons, we skip 1st letter, to avoid dealing with
- changes in capitalization in kernel. For example, I see that
- current kernel git (at 2011-01-01) has both "kernel BUG at ..."
- and "Kernel BUG at ..." messages, and I don't want to change
- the code below whenever kernel is changed to use "K" (or "k")
- uniformly.
- */
+static const char *const s_suspicious_strings[] = {
- "BUG:",
- "WARNING: at",
- "INFO: possible recursive locking detected",
- /*k*/"ernel BUG at",
- "list_del corruption",
- "list_add corruption",
- "do_IRQ: stack overflow:",
- /*n*/"ear stack overflow (cur:",
- /*g*/"eneral protection fault",
- /*u*/"nable to handle kernel",
- /*d*/"ouble fault:",
- "RTNL: assertion failed",
- /*e*/"eek! page_mapcount(page) went negative!",
- /*b*/"adness at",
- "NETDEV WATCHDOG",
- /*s*/"ysctl table check failed",
- ": nobody cared",
- "IRQ handler type mismatch",
- /* X86 TRAPs */
- "divide error:",
- "bounds:",
- "coprocessor segment overrun:",
- "invalid TSS:",
- "segment not present:",
- "invalid opcode:",
- "alignment check:",
- "stack segment:",
- "fpu exception:",
- "simd exception:",
- "iret exception:",
- /* Termination */
- NULL
+};
void koops_print_suspicious_strings(void) {
- fputs(
- "BUG" "\n"
- "corruption" "\n"
- "stack overflow" "\n"
- "protection fault" "\n"
- "WARNING: at" "\n"
- /*u*/ "nable to handle" "\n"
- /*d*/ "ouble fault:" "\n"
- "RTNL: assertion failed" "\n"
- /*e*/ "eek! page_mapcount(page) went negative!" "\n"
- /*b*/ "adness at" "\n"
- "NETDEV WATCHDOG" "\n"
- /*s*/ "ysctl table check failed" "\n"
- "INFO: possible recursive locking detected" "\n"
- ": nobody cared" "\n"
- "IRQ handler type mismatch" "\n"
- , stdout);
- const char *const *str = s_suspicious_strings;
- while (*str)
- {
puts(*str);
++str;
- }
}
why not use
for (; *str; ++str) {}
or may be even better (and you have here remove NULL from array)
for (int i = 0; i < ARRAY_SIZE(arr); i++) {}
in any case of mentioned above, code will be more readable and the rest of the code is in the same flavor.
void koops_extract_oopses(GList **oops_list, char *buffer, size_t buflen) @@ -216,55 +249,16 @@ next_line: if (oopsstart < 0) { /* Find start-of-oops markers */
/* In some comparisons, we skip 1st letter, to avoid dealing with
* changes in capitalization in kernel. For example, I see that
* current kernel git (at 2011-01-01) has both "kernel BUG at ..."
* and "Kernel BUG at ..." messages, and I don't want to change
* the code below whenever kernel is changed to use "K" (or "k")
* uniformly.
*/
if (strstr(curline, /*g*/ "eneral protection fault:"))
oopsstart = i;
else if (strstr(curline, "BUG:"))
oopsstart = i;
else if (strstr(curline, /*k*/ "ernel BUG at"))
oopsstart = i;
/* WARN_ON() generated message */
else if (strstr(curline, "WARNING: at "))
oopsstart = i;
else if (strstr(curline, /*u*/ "nable to handle kernel"))
oopsstart = i;
else if (strstr(curline, /*d*/ "ouble fault:"))
oopsstart = i;
else if (strstr(curline, "do_IRQ: stack overflow:"))
oopsstart = i;
else if (strstr(curline, "RTNL: assertion failed"))
oopsstart = i;
else if (strstr(curline, /*e*/ "eek! page_mapcount(page) went negative!"))
oopsstart = i;
else if (strstr(curline, /*n*/ "ear stack overflow (cur:"))
oopsstart = i;
else if (strstr(curline, /*b*/ "adness at"))
oopsstart = i;
else if (strstr(curline, "NETDEV WATCHDOG"))
oopsstart = i;
else if (strstr(curline, /*s*/ "ysctl table check failed"))
oopsstart = i;
else if (strstr(curline, "INFO: possible recursive locking detected"))
oopsstart = i;
// Not needed: "--[ cut here ]--" is always followed
// by "Badness at", "kernel BUG at", or "WARNING: at" string
//else if (strstr(curline, "------------[ cut here ]------------"))
// oopsstart = i;
else if (strstr(curline, "list_del corruption"))
oopsstart = i;
else if (strstr(curline, "list_add corruption"))
oopsstart = i;
/* "irq NN: nobody cared..." */
else if (strstr(curline, ": nobody cared"))
oopsstart = i;
else if (strstr(curline, "IRQ handler type mismatch"))
oopsstart = i;
const char *const *str = s_suspicious_strings;
while (*str)
{
if (strstr(curline, *str))
{
oopsstart = i;
break;
}
++str;
} if (oopsstart >= 0) {
@@ -335,14 +329,23 @@ next_line: oopsend = i-1; else if (strstr(curline, "Instruction dump:")) oopsend = i;
/* if a new oops starts, this one has ended */
else if (strstr(curline, "WARNING: at ") && oopsstart != i) /* WARN_ON() generated message */
oopsend = i-1;
else if (strstr(curline, "Unable to handle") && oopsstart != i)
oopsend = i-1; /* kernel end-of-oops marker (not including marker itself) */ else if (strstr(curline, "---[ end trace")) oopsend = i-1;
else
{
/* if a new oops starts, this one has ended */
const char *const *str = s_suspicious_strings;
while (*str)
{
if (strstr(curline, *str))
{
oopsend = i-1;
break;
}
++str;
}
} if (oopsend <= i) {