From: Vadim Zeitlin <vadim@wxwidgets.org>
Date: Tue, 18 Aug 2009 01:22:48 +0000 (+0000)
Subject: Fix last error display in wxLogSysError().
X-Git-Url: https://git.saurik.com/wxWidgets.git/commitdiff_plain/b804f9924d38b16d5cd72ba6c55195f9b1cc5896?ds=sidebyside

Fix last error display in wxLogSysError().

After recent changes of wxLogXXX() functions into macros the last error was
overwritten by wxString::Format() called between the call to wxLogSysError()
and wxLog::CallDoLogNow() which called wxSysErrorCode() and so its original
value was lost and, unless the last error was specified explicitly, it always
came out as 0.

To fix this, call wxSysErrorCode() directly when calling wxLogSysError(). This
may be unnecessary (if the error is given explicitly) but there doesn't seem
to be any other way to fix it and the overhead of calling wxSysErrorCode()
shouldn't be that big.

Also add a unit test checking that wxLogSysError() behaves as expected.

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@61692 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775
---

diff --git a/include/wx/log.h b/include/wx/log.h
index 7e0912c0da..59548bcbec 100644
--- a/include/wx/log.h
+++ b/include/wx/log.h
@@ -863,12 +863,12 @@ public:
     // indicates that we may have an extra first argument preceding the format
     // string and that if we do have it, we should store it in m_info using the
     // given key (while by default 0 value will be used)
-    wxLogger& MaybeStore(const wxString& key)
+    wxLogger& MaybeStore(const wxString& key, wxUIntPtr value = 0)
     {
         wxASSERT_MSG( m_optKey.empty(), "can only have one optional value" );
         m_optKey = key;
 
-        m_info.StoreValue(key, 0);
+        m_info.StoreValue(key, value);
         return *this;
     }
 
@@ -1207,7 +1207,7 @@ private:
 
     void DoCallOnLog(const wxString& format, va_list argptr)
     {
-        wxLog::OnLog(m_level, wxString::FormatV(format, argptr), m_info);
+        DoCallOnLog(m_level, format, argptr);
     }
 
 
@@ -1354,19 +1354,30 @@ WXDLLIMPEXP_BASE const wxChar* wxSysErrorMsg(unsigned long nErrCode = 0);
 // wxLogSysError() needs to stash the error code value in the log record info
 // so it needs special handling too; additional complications arise because the
 // error code may or not be present as the first argument
+//
+// notice that we unfortunately can't avoid the call to wxSysErrorCode() even
+// though it may be unneeded if an explicit error code is passed to us because
+// the message might not be logged immediately (e.g. it could be queued for
+// logging from the main thread later) and so we can't to wait until it is
+// logged to determine whether we have last error or not as it will be too late
+// and it will have changed already by then (in fact it even changes when
+// wxString::Format() is called because of vsnprintf() inside it so it can
+// change even much sooner)
 #define wxLOG_KEY_SYS_ERROR_CODE "wx.sys_error"
 
 #define wxLogSysError                                                         \
     if ( !wxLog::IsLevelEnabled(wxLOG_Error, wxLOG_COMPONENT) )               \
     {}                                                                        \
     else                                                                      \
-        wxMAKE_LOGGER(Error).MaybeStore(wxLOG_KEY_SYS_ERROR_CODE).Log
+        wxMAKE_LOGGER(Error).MaybeStore(wxLOG_KEY_SYS_ERROR_CODE,             \
+                                        wxSysErrorCode()).Log
 
 // unfortunately we can't have overloaded macros so we can't define versions
 // both with and without error code argument and have to rely on LogV()
 // overloads in wxLogger to select between them
 #define wxVLogSysError \
-    wxMAKE_LOGGER(Error).MaybeStore(wxLOG_KEY_SYS_ERROR_CODE).LogV
+    wxMAKE_LOGGER(Error).MaybeStore(wxLOG_KEY_SYS_ERROR_CODE, \
+                                    wxSysErrorCode()).LogV
 
 #if wxUSE_GUI
     // wxLogStatus() is similar to wxLogSysError() as it allows to optionally
diff --git a/src/common/log.cpp b/src/common/log.cpp
index 1d3ae8fffa..da627fec24 100644
--- a/src/common/log.cpp
+++ b/src/common/log.cpp
@@ -350,9 +350,7 @@ wxLog::CallDoLogNow(wxLogLevel level,
     wxUIntPtr num = 0;
     if ( info.GetNumValue(wxLOG_KEY_SYS_ERROR_CODE, &num) )
     {
-        long err = static_cast<long>(num);
-        if ( !err )
-            err = wxSysErrorCode();
+        const long err = static_cast<long>(num);
 
         suffix.Printf(_(" (error %ld: %s)"), err, wxSysErrorMsg(err));
     }
diff --git a/tests/log/logtest.cpp b/tests/log/logtest.cpp
index 644a18f89f..7d859c7d00 100644
--- a/tests/log/logtest.cpp
+++ b/tests/log/logtest.cpp
@@ -168,6 +168,7 @@ private:
         CPPUNIT_TEST( CompatLogger );
         CPPUNIT_TEST( CompatLogger2 );
 #endif // WXWIN_COMPATIBILITY_2_8
+        CPPUNIT_TEST( SysError );
     CPPUNIT_TEST_SUITE_END();
 
     void Functions();
@@ -180,6 +181,7 @@ private:
     void CompatLogger();
     void CompatLogger2();
 #endif // WXWIN_COMPATIBILITY_2_8
+    void SysError();
 
     TestLog *m_log;
     wxLog *m_logOld;
@@ -335,3 +337,21 @@ void LogTestCase::CompatLogger2()
 }
 
 #endif // WXWIN_COMPATIBILITY_2_8
+
+void LogTestCase::SysError()
+{
+    wxString s;
+    wxLogSysError("Success");
+    CPPUNIT_ASSERT( m_log->GetLog(wxLOG_Error).StartsWith("Success (", &s) );
+    CPPUNIT_ASSERT( s.StartsWith("error 0") );
+
+    wxLogSysError(17, "Error");
+    CPPUNIT_ASSERT( m_log->GetLog(wxLOG_Error).StartsWith("Error (", &s) );
+    CPPUNIT_ASSERT( s.StartsWith("error 17") );
+
+    wxOpen("no-such-file", 0, 0);
+    wxLogSysError("Not found");
+    CPPUNIT_ASSERT( m_log->GetLog(wxLOG_Error).StartsWith("Not found (", &s) );
+    WX_ASSERT_MESSAGE( ("Error message is \"(%s\"", s), s.StartsWith("error 2") );
+}
+