On Wed, 2010-11-24 at 15:29 +0100, Nikola Pajkovsky wrote:
- char tainted[2] = {'x', '\0'};
+ char tainted_str[sizeof(unsigned long) * 3 + 1] = {'\0'};
The code later does not try to convert the contents in any way,
so having exactly sizeof(unsigned long) * 3 + 1 bytes is unimpotant,
and is even a bit misleading - the reader of the code will be let
to think we plan to parse the content. We don't.
Therefore I'd just use "char tainted_str[SOME_NUM];"
or even use FILE-based IO - it has a nice "read one line, however long
it is" function:
FILE *fp = fopen...
if (fp) {
char *tainted_str = xmalloc_fgetline(fp);
...
free(tainted_str);
fclose(fp);
}
No need to think "how big char tainted_str[SOME_NUM] should be?"
/* once tainted flag is set to 1, only restart can reset the
flag to 0 */
int tainted_fd = open("/proc/sys/kernel/tainted", O_RDONLY);
if (tainted_fd >= 0)
{
- /* contain only 0 or 1 */
- if (read(tainted_fd, &tainted, 1) != 1)
+ /* this file contains a decimal representation of a bitmask */
This comment is not needed. Yes, file contains a number.
But it is not relevant *for this piece of code*.
+ if (read(tainted_fd, tainted_str, sizeof(tainted_str)) ==
-1)
There will be no NUL byte if read gets exactly sizeof(tainted_str)
bytes. Use sizeof(tainted_str)-1.
error_msg("Unable to read one byte from
/proc/sys/kernel/tainted");
-
The error message needs changing too.
I'd go with simple "Can't read /proc/sys/kernel/tainted".
@@ -158,8 +157,8 @@ int save_oops_to_debug_dump(GList **oopsList)
strchrnul(second_line, '\n')[0] = '\0';
dd_save_text(dd, FILENAME_REASON, second_line);
- if (tainted[0] == '1')
- dd_save_text(dd, FILENAME_TAINTED, tainted);
+ if (tainted_str[0])
+ dd_save_text(dd, FILENAME_TAINTED, tainted_str);
Might also do not write the file if tainted_str == "0":
if (tainted_str[0] && tainted_str[0] != '0') ...
diff --git a/src/plugins/abrt-action-bugzilla.cpp
b/src/plugins/abrt-action-bugzilla.cpp
index 073d534..cdbe344 100644
--- a/src/plugins/abrt-action-bugzilla.cpp
+++ b/src/plugins/abrt-action-bugzilla.cpp
@@ -382,6 +382,68 @@ int ctx::add_comment(xmlrpc_int32 bug_id, const char* comment, bool
is_private)
return result ? 0 : -1;
}
+/* From RHEL6 kernel/panic.c:
+ * { TAINT_PROPRIETARY_MODULE, 'P', 'G' },
+ * { TAINT_FORCED_MODULE, 'F', ' ' },
+ * { TAINT_UNSAFE_SMP, 'S', ' ' },
+ * { TAINT_FORCED_RMMOD, 'R', ' ' },
+ * { TAINT_MACHINE_CHECK, 'M', ' ' },
+ * { TAINT_BAD_PAGE, 'B', ' ' },
+ * { TAINT_USER, 'U', ' ' },
+ * { TAINT_DIE, 'D', ' ' },
+ * { TAINT_OVERRIDDEN_ACPI_TABLE, 'A', ' ' },
+ * { TAINT_WARN, 'W', ' ' },
+ * { TAINT_CRAP, 'C', ' ' },
+ * { TAINT_FIRMWARE_WORKAROUND, 'I', ' ' },
+ * entries 12 - 27 are unused
+ * { TAINT_HARDWARE_UNSUPPORTED, 'H', ' ' },
+ * entries 29 - 31 are unused
+ */
+
+static const char *taint_warnings[] = {
static const char *const taint_warnings[]
+ "Proprietary Module",
+ "Forced Module",
+ "Unsafe SMP",
+ "Forced rmmod",
+ "Machine Check",
+ "Bad Page",
+ "User",
+ "Die",
+ "Overriden ACPI Table",
+ "Warning Issued",
+ "Experimental Module Loaded",
+ "Firmware Workaround",
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ "Hardware Unsupported",
+ NULL,
+ NULL,
+};
+
+static const char *tainted_string(unsigned long tainted)
+{
+ unsigned long idx = 0;
Why "long"?
+ while (tainted >>= 1)
+ idx++;
+
+ return taint_warnings[idx];
+}
+
xmlrpc_int32 ctx::new_bug(const map_crash_data_t& pCrashData, int depend_on_bugno)
{
const char *package = get_crash_data_item_content_or_NULL(pCrashData,
FILENAME_PACKAGE);
@@ -392,7 +454,7 @@ xmlrpc_int32 ctx::new_bug(const map_crash_data_t& pCrashData, int
depend_on_bugn
const char *reason = get_crash_data_item_content_or_NULL(pCrashData,
FILENAME_REASON);
const char *function = get_crash_data_item_content_or_NULL(pCrashData,
FILENAME_CRASH_FUNCTION);
const char *analyzer = get_crash_data_item_content_or_NULL(pCrashData,
FILENAME_ANALYZER);
- const char *tainted = get_crash_data_item_content_or_NULL(pCrashData,
FILENAME_TAINTED);
+ const char *tainted_str = get_crash_data_item_content_or_NULL(pCrashData,
FILENAME_TAINTED);
struct strbuf *buf_summary = strbuf_new();
strbuf_append_strf(buf_summary, "[abrt] %s", package);
@@ -403,11 +465,13 @@ xmlrpc_int32 ctx::new_bug(const map_crash_data_t& pCrashData,
int depend_on_bugn
if (reason != NULL)
strbuf_append_strf(buf_summary, ": %s", reason);
- if (tainted && analyzer
- && (tainted[0] == '1')
+ if (tainted_str && analyzer
&& (strcmp(analyzer, "Kerneloops") == 0)
) {
- strbuf_append_str(buf_summary, ": TAINTED");
+ unsigned long tainted = xatoi_u(tainted_str);
+ const char *tainted_warning = tainted_string(tainted);
+ if (tainted_warning)
+ strbuf_append_strf(buf_summary, ": TAINTED %s", tainted_warning);
}
char *status_whiteboard = xasprintf("abrt_hash:%s", duphash);
--
vda