simplify kerneloops/dumpoops a bit

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

2 files changed, 101 insertions, 122 deletions

diff --git a/lib/Plugins/KerneloopsScanner.cpp b/lib/Plugins/KerneloopsScanner.cpp
index 4a264b02..f30906ac 100644
--- a/lib/Plugins/KerneloopsScanner.cpp
+++ b/lib/Plugins/KerneloopsScanner.cpp
@@ -29,103 +29,30 @@
 
 // TODO: https://fedorahosted.org/abrt/ticket/78
 
-CKerneloopsScanner::CKerneloopsScanner()
-{
-	int cnt_FoundOopses;
-
-	/* Scan dmesg, on first call only */
-	cnt_FoundOopses = ScanDmesg();
-	if (cnt_FoundOopses > 0)
-		SaveOopsToDebugDump();
-}
-
-void CKerneloopsScanner::Run(const char *pActionDir, const char *pArgs, int force)
-{
-	const char *syslog_file = "/var/log/messages";
-	map_plugin_settings_t::const_iterator it = m_pSettings.find("SysLogFile");
-	if (it != m_pSettings.end()) {
-		syslog_file = it->second.c_str();
-	}
-
-	int cnt_FoundOopses = ScanSysLogFile(syslog_file);
-	if (cnt_FoundOopses > 0) {
-		SaveOopsToDebugDump();
-		/*
-		 * This marker in syslog file prevents us from
-		 * re-parsing old oopses (any oops before it is
-		 * ignored by ScanSysLogFile()). The only problem
-		 * is that we can't be sure here that syslog_file
-		 * is the file where syslog(xxx) stuff ends up.
-		 */
-		openlog("abrt", 0, LOG_KERN);
-		syslog(
-			LOG_WARNING,
-			"Kerneloops: Reported %u kernel oopses to Abrt",
-			cnt_FoundOopses
-		);
-		closelog();
-	}
-}
-
-void CKerneloopsScanner::SaveOopsToDebugDump()
-{
-	update_client(_("Creating kernel oops crash reports..."));
-
-	int countdown = 16; /* do not report hundreds of oopses */
-	time_t t = time(NULL);
-	vector_string_t oopsList = m_pOopsList;
-	m_pOopsList.clear();
-
-	while (!oopsList.empty() && --countdown != 0) {
-		char path[sizeof(DEBUG_DUMPS_DIR"/kerneloops-%lu-%lu") + 2 * sizeof(long)*3];
-		sprintf(path, DEBUG_DUMPS_DIR"/kerneloops-%lu-%lu",
-				(long)t, (long)oopsList.size());
-
-		std::string oops = oopsList.back();
-		const char *first_line = oops.c_str();
-		char *second_line = (char*)strchr(first_line, '\n'); /* never NULL */
-              	*second_line++ = '\0';
-		try
-		{
-			CDebugDump dd;
-			dd.Create(path, 0);
-			dd.SaveText(FILENAME_ANALYZER, "Kerneloops");
-			dd.SaveText(FILENAME_EXECUTABLE, "kernel");
-			dd.SaveText(FILENAME_KERNEL, first_line);
-			dd.SaveText(FILENAME_PACKAGE, "not_applicable");
-			dd.SaveText(FILENAME_CMDLINE, "not_applicable");
-			dd.SaveText(FILENAME_COMPONENT, "kernel");
-			dd.SaveText(FILENAME_KERNELOOPS, second_line);
-		}
-		catch (CABRTException& e)
-		{
-			throw CABRTException(EXCEP_PLUGIN, "%s: %s", __func__, e.what());
-		}
-		oopsList.pop_back();
-	}
-}
-
-int CKerneloopsScanner::ScanDmesg()
+static int scan_dmesg(vector_string_t& oopsList)
 {
-	VERB1 log("Scanning dmesg...");
-
-	int cnt_FoundOopses;
-	char *buffer;
-	long pagesz = sysconf(_SC_PAGESIZE);
-
-	buffer = (char*)xzalloc(pagesz + 1);
+	VERB1 log("Scanning dmesg");
 
-	syscall(__NR_syslog, 3, buffer, pagesz);
-	m_pOopsList.clear();
-	cnt_FoundOopses = extract_oopses(m_pOopsList, buffer, strlen(buffer));
+	/* syslog(3) - read the last len bytes from the log buffer
+	 * (non-destructively), but dont read more than was written
+	 * into the buffer since the last"clear ring buffer" cmd.
+	 * Returns the number of bytes read.
+	 */
+	char *buffer = (char*)xzalloc(16*1024);
+	syscall(__NR_syslog, 3, buffer, 16*1024 - 1); /* always NUL terminated */
+	int cnt_FoundOopses = extract_oopses(oopsList, buffer, strlen(buffer));
 	free(buffer);
 
 	return cnt_FoundOopses;
 }
 
-int CKerneloopsScanner::ScanSysLogFile(const char *filename)
+
+/* "dumpoops" tool uses these two functions too */
+extern "C" {
+
+int scan_syslog_file(vector_string_t& oopsList, const char *filename)
 {
-	VERB1 log("Scanning syslog...");
+	VERB1 log("Scanning syslog file '%s'", filename);
 
 	char *buffer;
 	struct stat statb;
@@ -143,9 +70,9 @@ int CKerneloopsScanner::ScanSysLogFile(const char *filename)
 	}
 
 	/*
-	 * in theory there's a race here, since someone could spew
+	 * In theory we have a race here, since someone could spew
 	 * to /var/log/messages before we read it in... we try to
-	 * deal with it by reading at most 1023 bytes extra. If there's
+	 * deal with it by reading at most 10kbytes extra. If there's
 	 * more than that.. any oops will be in dmesg anyway.
 	 * Do not try to allocate an absurd amount of memory; ignore
 	 * older log messages because they are unlikely to have
@@ -153,9 +80,9 @@ int CKerneloopsScanner::ScanSysLogFile(const char *filename)
 	 * than enough; it's not worth looping through more log
 	 * if the log is larger than that.
 	 */
-	sz = statb.st_size + 1024;
-	if (statb.st_size > (32*1024*1024 - 1024)) {
-		xlseek(fd, statb.st_size - (32*1024*1024 - 1024), SEEK_SET);
+	sz = statb.st_size + 10*1024;
+	if (statb.st_size > (32*1024*1024 - 10*1024)) {
+		xlseek(fd, statb.st_size - (32*1024*1024 - 10*1024), SEEK_SET);
 		sz = 32*1024*1024;
 	}
 	buffer = (char*)xzalloc(sz);
@@ -164,34 +91,97 @@ int CKerneloopsScanner::ScanSysLogFile(const char *filename)
 
 	cnt_FoundOopses = 0;
 	if (sz > 0) {
-		m_pOopsList.clear();
-		cnt_FoundOopses = extract_oopses(m_pOopsList, buffer, sz);
+		cnt_FoundOopses = extract_oopses(oopsList, buffer, sz);
 	}
 	free(buffer);
 
 	return cnt_FoundOopses;
 }
 
-PLUGIN_INFO(ACTION,
-            CKerneloopsScanner,
-            "KerneloopsScanner",
-            "0.0.1",
-            "Periodically scans for and saves kernel oopses",
-            "anton@redhat.com",
-            "http://people.redhat.com/aarapov",
-            "");
+void save_oops_to_debug_dump(const vector_string_t& oopsList)
+{
+	unsigned countdown = 16; /* do not report hundreds of oopses */
+	unsigned idx = oopsList.size();
+	time_t t = time(NULL);
+
+	VERB1 log("Saving %u oopses as crash dump dirs", idx >= countdown ? countdown-1 : idx);
+
+	while (idx != 0 && --countdown != 0) {
+		char path[sizeof(DEBUG_DUMPS_DIR"/kerneloops-%lu-%lu") + 2 * sizeof(long)*3];
+		sprintf(path, DEBUG_DUMPS_DIR"/kerneloops-%lu-%lu", (long)t, (long)idx);
+		try
+		{
+			std::string oops = oopsList.at(--idx);
+			const char *first_line = oops.c_str();
+			char *second_line = (char*)strchr(first_line, '\n'); /* never NULL */
+              		*second_line++ = '\0';
+
+			CDebugDump dd;
+			dd.Create(path, /*uid:*/ 0);
+			dd.SaveText(FILENAME_ANALYZER, "Kerneloops");
+			dd.SaveText(FILENAME_EXECUTABLE, "kernel");
+			dd.SaveText(FILENAME_KERNEL, first_line);
+			dd.SaveText(FILENAME_PACKAGE, "not_applicable");
+			dd.SaveText(FILENAME_CMDLINE, "not_applicable");
+			dd.SaveText(FILENAME_COMPONENT, "kernel");
+			dd.SaveText(FILENAME_KERNELOOPS, second_line);
+		}
+		catch (CABRTException& e)
+		{
+			throw CABRTException(EXCEP_PLUGIN, "%s: %s", __func__, e.what());
+		}
+	}
+}
+
+} /* extern "C" */
 
-/* For "dumpoops" tool */
-extern "C" {
 
-int scan_syslog_file(CKerneloopsScanner *This, const char *filename)
+CKerneloopsScanner::CKerneloopsScanner()
 {
-	return This->ScanSysLogFile(filename);
+	int cnt_FoundOopses;
+
+	/* Scan dmesg, on first call only */
+	vector_string_t oopsList;
+	cnt_FoundOopses = scan_dmesg(oopsList);
+	if (cnt_FoundOopses > 0) {
+		save_oops_to_debug_dump(oopsList);
+	}
 }
 
-void save_oops_to_debug_dump(CKerneloopsScanner *This)
+void CKerneloopsScanner::Run(const char *pActionDir, const char *pArgs, int force)
 {
-	This->SaveOopsToDebugDump();
+	const char *syslog_file = "/var/log/messages";
+	map_plugin_settings_t::const_iterator it = m_pSettings.find("SysLogFile");
+	if (it != m_pSettings.end()) {
+		syslog_file = it->second.c_str();
+	}
+
+	vector_string_t oopsList;
+	int cnt_FoundOopses = scan_syslog_file(oopsList, syslog_file);
+	if (cnt_FoundOopses > 0) {
+		save_oops_to_debug_dump(oopsList);
+		/*
+		 * This marker in syslog file prevents us from
+		 * re-parsing old oopses (any oops before it is
+		 * ignored by scan_syslog_file()). The only problem
+		 * is that we can't be sure here that syslog_file
+		 * is the file where syslog(xxx) stuff ends up.
+		 */
+		openlog("abrt", 0, LOG_KERN);
+		syslog(
+			LOG_WARNING,
+			"Kerneloops: Reported %u kernel oopses to Abrt",
+			cnt_FoundOopses
+		);
+		closelog();
+	}
 }
 
-} /* extern "C" */
+PLUGIN_INFO(ACTION,
+            CKerneloopsScanner,
+            "KerneloopsScanner",
+            "0.0.1",
+            "Periodically scans for and saves kernel oopses",
+            "anton@redhat.com",
+            "http://people.redhat.com/aarapov",
+            "");
diff --git a/lib/Plugins/KerneloopsScanner.h b/lib/Plugins/KerneloopsScanner.h
index dfcb8b5e..da856e04 100644
--- a/lib/Plugins/KerneloopsScanner.h
+++ b/lib/Plugins/KerneloopsScanner.h
@@ -33,17 +33,6 @@
 
 class CKerneloopsScanner : public CAction
 {
-		/* For "dumpoops" tool */
-	public:
-		vector_string_t m_pOopsList;
-
-		/* For "dumpoops" tool */
-	public:
-		void SaveOopsToDebugDump();
-		int ScanDmesg();
-		int ScanSysLogFile(const char *filename);
-
-		/* Plugin interface */
 	public:
 		CKerneloopsScanner();
 		virtual void Run(const char *pActionDir, const char *pArgs, int force);