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 7e0912c0da1d8e3a0b46e53378d98d8a288202e9..59548bcbeca11007e891004b4f09b00a76ee8bce 100644 (file)
--- 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 1d3ae8fffa51773193970140b26e6dab75d8b46f..da627fec24e600457254e9747375766457419b91 100644 (file)
--- 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 644a18f89f189ca701090d67770e32e50b3fa90b..7d859c7d0047bee58facf56b4aa207883a5e8230 100644 (file)
--- 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") );
+}
+