Issue #1234 - Rework strings in debug facility

src/plugin/PluginDebug.cpp

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 #include "PluginStdAfx.h"
 
 #include "PluginDebug.h"
 #include "PluginMutex.h"
 #include "PluginSettings.h"
 
-
 class CPluginDebugLock : public CPluginMutex
 {
 
 private:
 
   static CComAutoCriticalSection s_criticalSectionDebugLock;
 
 public:
 
   CPluginDebugLock() : CPluginMutex(L"DebugFile", PLUGIN_ERROR_MUTEX_DEBUG_FILE)
   {
     s_criticalSectionDebugLock.Lock();
   }
 
   ~CPluginDebugLock()
   {
     s_criticalSectionDebugLock.Unlock();
   }
 };
 
 CComAutoCriticalSection CPluginDebugLock::s_criticalSectionDebugLock;
 
+void CPluginDebug::DebugSystemException(const std::system_error& ex, int errorId, int errorSubid, const std::string& description)
+{
+  std::string message = description + ", " + ex.code().message() + ", " + ex.what();
+  DEBUG_ERROR_LOG(ex.code().value(), errorId, errorSubid, message);
+}
 
 #ifdef ENABLE_DEBUG_INFO
 
-void CPluginDebug::Debug(const CString& text, DWORD dwProcessId, DWORD dwThreadId)
+void DebugLegacy(const CString& text, DWORD dwProcessId, DWORD dwThreadId)

[Oleksandr, 2015/02/27 07:50:15]

Isn't this changeset about converting CStrings to std strings in Debug
functions? Why do we need this legacy function?

[Eric, 2015/02/27 14:26:56]

It is, but I've split the work into two parts. First, in the present review,
I've changed just the API that these debug functions have, switching from
CString to std::wstring. Second, in work yet to be done, we'll rewrite the
internals of these debug functions to eliminate CString.

The trouble is that we don't have drop-in replacement functions for some of the
CString member functions. What that means is lots of rewriting and/or new
utility function. That would have increased the size of this change set too
much, so I split up this change into two parts.

 {
 #ifdef USE_CONSOLE
   CONSOLE("%s", CT2A(text.GetString(), CP_UTF8));
 #endif
 
   if (CPluginSettings::HasInstance())
   {
 #ifdef ENABLE_DEBUG_SPLIT_FILE
     CPluginSettings* settings = CPluginSettings::GetInstance();
 
@@ skipping 13 matching lines @@
     }
     if (dwThreadId == 0)
     {
       dwThreadId = ::GetCurrentThreadId();
     }
 
     CStringA processInfo;
     processInfo.Format("%4.4u.%4.4u - ", dwProcessId, dwThreadId);
 #endif
     SYSTEMTIME st;
     ::GetSystemTime(&st);

[Oleksandr, 2015/02/27 07:50:15]

Probably easier to replace this with std::localtime(std::time(nullptr)) and then
use std::strftime

[Eric, 2015/02/27 14:26:56]

Yes, we can do that when we rewrite this implementation.

 
     CStringA sysTime;
     sysTime.Format("%2.2d:%2.2d:%2.2d.%3.3d - ", st.wHour, st.wMinute, st.wSecond, st.wMilliseconds);
 
     CPluginDebugLock lock;
     if (lock.IsLocked())
     {
       std::ofstream debugFile;
 
 #ifdef ENABLE_DEBUG_SPLIT_FILE
       debugFile.open(GetDataPath(L"debug_" + processor + L".txt"), std::ios::app);
 #else
       debugFile.open(GetDataPath(L"debug.txt"), std::ios::app);
 #endif
       int pos = 0;
       CStringA line = text.Tokenize(L"\n\r", pos);

[Oleksandr, 2015/02/27 07:50:15]

Looks like just a place for wcstok_s again

[Eric, 2015/02/27 14:26:56]

Actually, we don't need to tokenize at all here. The only reason to tokenize at
all is to deal with multiple-line messages. What I did last summer was to add a
function that accepted arrays of strings; that worked just fine. I thought it
was clearer to show array brackets than newlines within strings to show what was
happening.

In any case, it's an issue for stage two, where we rewrite the internals of
these functions.

 
       while (pos >= 0)
       {
         debugFile.write(sysTime.GetBuffer(), sysTime.GetLength());
 #ifndef ENABLE_DEBUG_SPLIT_FILE
         debugFile.write(processInfo.GetBuffer(), processInfo.GetLength());
 #endif
         debugFile.write(line.GetBuffer(), line.GetLength());
         debugFile.write("\n", 1);
 
         line = text.Tokenize(L"\n\r", pos);
       }
 
       debugFile.flush();
     }
   }
 }
 
-void CPluginDebug::DebugClear()
+void CPluginDebug::Debug(const std::string& text, DWORD processId, DWORD threadId)
 {
-  CPluginDebugLock lock;
-  if (lock.IsLocked())
-  {
-    DeleteFileW(GetDataPath(L"debug.txt").c_str());
-    DeleteFileW(GetDataPath(L"debug_main_ui.txt").c_str());
-    DeleteFileW(GetDataPath(L"debug_main_thread.txt").c_str());
+  DebugLegacy(CString(text.c_str()), processId, threadId);
+}
 
-    for (int i = 1; i <= 10; i++)
-    {
-      std::wstring x = std::to_wstring(i);
-      DeleteFileW(GetDataPath(L"debug_tab" + x + L"_ui.txt").c_str());
-      DeleteFileW(GetDataPath(L"debug_tab" + x + L"_thread.txt").c_str());
-    }
-  }
+void CPluginDebug::Debug(const std::wstring& text, DWORD processId, DWORD threadId)
+{
+  DebugLegacy(ToCString(text), processId, threadId);
 }
 
 #endif
 
-#if (defined ENABLE_DEBUG_INFO || defined ENABLE_DEBUG_SELFTEST)
+#if (defined ENABLE_DEBUG_INFO)
 
-void CPluginDebug::DebugError(const CString& error)
+void CPluginDebug::DebugException(const std::exception& ex)
 {
+  auto error = std::string("!!! Exception:") + ex.what();
 #ifdef ENABLE_DEBUG_ERROR
   Debug(error);
 #endif
 
   DEBUG_SELFTEST("********************************************************************************\n" + error + "\n********************************************************************************")
 }
 
-void CPluginDebug::DebugErrorCode(DWORD errorCode, const CString& error, DWORD dwProcessId, DWORD dwThreadId)
+void DebugErrorCodeLegacy(DWORD errorCode, const CString& error, DWORD dwProcessId, DWORD dwThreadId)

[Oleksandr, 2015/02/27 07:50:15]

Same here. Why do we need legacy functions after this changeset?

[Eric, 2015/02/27 14:26:56]

See above. Same reason.

 {
   CString errorCodeText;
   errorCodeText.Format(L"%u (0x%8.8x)", errorCode, errorCode);
 
   CString finalError = error + L". error=" + errorCodeText;
 
 #ifdef ENABLE_DEBUG_ERROR
-  Debug(finalError, dwProcessId, dwThreadId);
+  DebugLegacy(finalError, dwProcessId, dwThreadId);
 #endif
 
-  DEBUG_SELFTEST(L"********************************************************************************\n" + finalError + "\n********************************************************************************")
+  DEBUG_SELFTEST(L"********************************************************************************\n" + finalError + "\ n********************************************************************************")
+}
+
+void CPluginDebug::DebugErrorCode(DWORD errorCode, const std::string& error, DWORD processId, DWORD threadId)
+{
+  DebugErrorCodeLegacy(errorCode, CString(error.c_str()), processId, threadId);
 }
 
 #endif
 
 // ============================================================================
 // Debug result
 // ============================================================================
 
 #ifdef ENABLE_DEBUG_RESULT
 
-void CPluginDebug::DebugResult(const CString& text)
+void DebugResultLegacy(const CString& text)
 {
   SYSTEMTIME st;
   ::GetSystemTime(&st);
 
   CStringA sysTime;
   sysTime.Format("%2.2d:%2.2d:%2.2d.%3.3d - ", st.wHour, st.wMinute, st.wSecond, st.wMilliseconds);
 
   CStringA textA = text;
 
   CPluginDebugLock lock;
   if (lock.IsLocked())
   {
     std::ofstream debugFile;
 
     debugFile.open(GetDataPath(L"debug_result.txt"), std::ios::app);
     debugFile.write(sysTime.GetBuffer(), sysTime.GetLength());
     debugFile.write(LPCSTR(textA), textA.GetLength());
     debugFile.write("\n", 1);
     debugFile.flush();
   }
 }
 
-void CPluginDebug::DebugResultDomain(const CString& domain)
+void CPluginDebug::DebugResult(const std::wstring& text)
+{
+  DebugResultLegacy(ToCString(text));
+}
+
+void CPluginDebug::DebugResultDomain(const std::wstring& domain)
 {
   DebugResult(L"===========================================================================================================================================================================================");
   DebugResult(domain);
   DebugResult(L"===========================================================================================================================================================================================");
 }
 
 
-void CPluginDebug::DebugResultBlocking(const CString& type, const std::wstring& src, const std::wstring& domain)
+void CPluginDebug::DebugResultBlocking(const std::wstring& type, const std::wstring& src, const std::wstring& domain)
 {
   CString srcTrunc = ToCString(src);
   if (src.length() > 100)
   {
     srcTrunc = srcTrunc.Left(67) + L"..." + srcTrunc.Right(30);
   }
 
   CString blocking;
-  blocking.Format(L"Blocked  %-12s  %-20s  %s", type, domain.empty()? L"-" : ToCString(domain), srcTrunc);
+  blocking.Format(L"Blocked  %-12s  %-20s  %s", ToCString(type), domain.empty()? L"-" : ToCString(domain), srcTrunc);
 
-  DebugResult(blocking);
+  DebugResultLegacy(blocking);
 }
 
 
-void CPluginDebug::DebugResultHiding(const CString& tag, const CString& id, const CString& filter)
+void CPluginDebug::DebugResultHiding(const std::wstring& tag, const std::wstring& id, const std::wstring& filter)
 {
-  CString srcTrunc = id;
+  CString srcTrunc = ToCString(id);
   if (srcTrunc.GetLength() > 100)
   {
     srcTrunc = srcTrunc.Left(67) + L"..." + srcTrunc.Right(30);
   }
 
   CString blocking;
-  blocking.Format(L"Hidden   %-12s  - %s  %s", tag, srcTrunc, filter);
+  blocking.Format(L"Hidden   %-12s  - %s  %s", ToCString(tag), srcTrunc, ToCString(filter));
 
-  DebugResult(blocking);
-}
-
-
-void CPluginDebug::DebugResultClear()
-{
-  CPluginDebugLock lock;
-  if (lock.IsLocked())
-  {
-    DeleteFileW(GetDataPath(L"debug_result.txt").c_str());
-  }
+  DebugResultLegacy(blocking);
 }
 
 #endif // ENABLE_DEBUG_RESULT
 
 
 #ifdef ENABLE_DEBUG_RESULT_IGNORED
 
-void CPluginDebug::DebugResultIgnoring(const CString& type, const std::wstring& src, const std::wstring& domain)
+void CPluginDebug::DebugResultIgnoring(const std::wstring& type, const std::wstring& src, const std::wstring& domain)
 {
   CString srcTrunc = ToCString(src);
   if (src.length() > 100)
   {
     srcTrunc = srcTrunc.Left(67) + L"..." + srcTrunc.Right(30);
   }
 
   CString blocking;
-  blocking.Format(L"Ignored  %-12s  %s  %s", type, domain.empty()? L"-" : ToCString(domain), srcTrunc);
+  blocking.Format(L"Ignored  %-12s  %s  %s", ToCString(type), domain.empty()? L"-" : ToCString(domain), srcTrunc);
 
-  DebugResult(blocking);
+  DebugResultLegacy(blocking);
 }
 
 #endif // ENABLE_DEBUG_RESULT_IGNORED