« previous next »
Pages: [1]   Go Down

Author Topic: logic error on the SupportsCurrentPlatform() related code  (Read 13590 times)

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 6025
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
logic error on the SupportsCurrentPlatform() related code
« on: April 08, 2015, 09:37:38 am »
@Morten
I see the recent changes in rev 10189 and rev 10194, debugged a little, I think the predefined macros in our CCDebugDialog is wrong.

Question:

Code
bool NativeParser::AddCompilerPredefinedMacrosGCC(const wxString& compilerId, cbProject* project, wxString& defs, ParserBase* parser)
{
    Compiler* compiler = CompilerFactory::GetCompiler(compilerId);
    if (!compiler)
        return false;

    if (   !parser->Options().platformCheck
        || (parser->Options().platformCheck && compiler->SupportsCurrentPlatform()) )
    {
        TRACE(_T("NativeParser::AddCompilerPredefinedMacrosGCC: Not supported on current platform!"));
        return false;
    }
Do you think there should be a ! after &&.

Also, if user don't want that platform check, do we need to "return false;"? I think in this case, we should run the old logic, or maybe, we need to remove the line:
Code
!parser->Options().platformCheck

Logged
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9702
Re: logic error on the SupportsCurrentPlatform() related code
« Reply #1 on: April 08, 2015, 09:46:12 am »
Looks like that on second thoughts... maybe I was drunk. :-\

Will you take the action?
Logged
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: https://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: https://wiki.codeblocks.org/index.php?title=FAQ

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 6025
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: logic error on the SupportsCurrentPlatform() related code
« Reply #2 on: April 08, 2015, 09:51:25 am »
Quote from: MortenMacFly on April 08, 2015, 09:46:12 am
Looks like that on second thoughts... maybe I was drunk. :-\

Will you take the action?
OK.
I will fix it soon.
Logged
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 6025
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: logic error on the SupportsCurrentPlatform() related code
« Reply #3 on: April 08, 2015, 10:32:31 am »
Fixed in trunk(r10198)
Logged
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline Huki

  • Multiple posting newcomer
  • *
  • Posts: 95
Re: logic error on the SupportsCurrentPlatform() related code
« Reply #4 on: April 08, 2015, 02:44:58 pm »
Oh yes, I discovered this error yesterday and added the same fix as in r10198.
Btw, on Linux there is another problem with backslash in: "\\bin\\", this has to be taken care too. Here is the entire patch:
Code
From dc6aa186413beff51b7a95f4010cd03cc1390c06 Mon Sep 17 00:00:00 2001
From: huki <gk7huki@gmail.com>
Date: Wed, 8 Apr 2015 02:02:25 +0530
Subject: CC: fix compiler predefined macros collection

---
 src/plugins/codecompletion/nativeparser.cpp | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/src/plugins/codecompletion/nativeparser.cpp b/src/plugins/codecompletion/nativeparser.cpp
index bf9113d..e17ede1 100644
--- a/src/plugins/codecompletion/nativeparser.cpp
+++ b/src/plugins/codecompletion/nativeparser.cpp
@@ -2000,8 +2000,7 @@ bool NativeParser::AddCompilerPredefinedMacrosGCC(const wxString& compilerId, cb
     if (!compiler)
         return false;
 
-    if (   !parser->Options().platformCheck
-        || (parser->Options().platformCheck && compiler->SupportsCurrentPlatform()) )
+    if ( parser->Options().platformCheck && !compiler->SupportsCurrentPlatform() )
     {
         TRACE(_T("NativeParser::AddCompilerPredefinedMacrosGCC: Not supported on current platform!"));
         return false;
@@ -2009,7 +2008,11 @@ bool NativeParser::AddCompilerPredefinedMacrosGCC(const wxString& compilerId, cb
 
     wxString masterPath = compiler->GetMasterPath();
     Manager::Get()->GetMacrosManager()->ReplaceMacros(masterPath);
+#ifdef __WXMSW__
     const wxString cpp_compiler = masterPath + _T("\\bin\\") + compiler->GetPrograms().CPP;
+#else
+    const wxString cpp_compiler = masterPath + _T("/bin/") + compiler->GetPrograms().CPP;
+#endif
     if ( !wxFileName::FileExists(cpp_compiler) )
         return false;
 
@@ -2116,8 +2119,7 @@ bool NativeParser::AddCompilerPredefinedMacrosVC(const wxString& compilerId, wxS
     if (!compiler)
         return false;
 
-    if (   !parser->Options().platformCheck
-        || (parser->Options().platformCheck && compiler->SupportsCurrentPlatform()) )
+    if ( parser->Options().platformCheck && !compiler->SupportsCurrentPlatform() )
     {
         TRACE(_T("NativeParser::AddCompilerPredefinedMacrosVC: Not supported on current platform!"));
         return false;
@@ -2125,7 +2127,11 @@ bool NativeParser::AddCompilerPredefinedMacrosVC(const wxString& compilerId, wxS
 
     wxString masterPath = compiler->GetMasterPath();
     Manager::Get()->GetMacrosManager()->ReplaceMacros(masterPath);
+#ifdef __WXMSW__
     const wxString c_compiler = masterPath + _T("\\bin\\") + compiler->GetPrograms().C;
+#else
+    const wxString c_compiler = masterPath + _T("/bin/") + compiler->GetPrograms().C;
+#endif
     if ( !wxFileName::FileExists(c_compiler) )
         return false;
 
-- 
1.9.1

Logged

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 6025
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: logic error on the SupportsCurrentPlatform() related code
« Reply #5 on: April 08, 2015, 03:38:50 pm »
Hi, Huki, thanks, before commit the fix to trunk, I just want to ask a simple question:
Code
@@ -2009,7 +2008,11 @@ bool NativeParser::AddCompilerPredefinedMacrosGCC(const wxString& compilerId, cb
 
     wxString masterPath = compiler->GetMasterPath();
     Manager::Get()->GetMacrosManager()->ReplaceMacros(masterPath);
+#ifdef __WXMSW__
     const wxString cpp_compiler = masterPath + _T("\\bin\\") + compiler->GetPrograms().CPP;
+#else
+    const wxString cpp_compiler = masterPath + _T("/bin/") + compiler->GetPrograms().CPP;
+#endif
     if ( !wxFileName::FileExists(cpp_compiler) )
         return false;
 
What was the problem if we don't have the above patch applied under Linux? Will wxFileName::FileExists() function call fail? If this does fail, then I believe this is a very long time bug. I don't have a Linux system at hand, so I can't verify myself. :)
Logged
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline Huki

  • Multiple posting newcomer
  • *
  • Posts: 95
Re: logic error on the SupportsCurrentPlatform() related code
« Reply #6 on: April 08, 2015, 05:10:15 pm »
Quote from: ollydbg on April 08, 2015, 03:38:50 pm
What was the problem if we don't have the above patch applied under Linux? Will wxFileName::FileExists() function call fail? If this does fail, then I believe this is a very long time bug. I don't have a Linux system at hand, so I can't verify myself. :)
Yes, the wxFileName::FileExists() call fails. I just tried skipping this check and keep the backward slash to see what happens: then the wxExecute does not report a failure, but no defines are collected (the defines list is empty). So it seems the wxExecute actually fails but it doesn't get reported.
By using forward slash for "/bin/" the defines are collected properly.
Logged

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 6025
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: logic error on the SupportsCurrentPlatform() related code
« Reply #7 on: April 09, 2015, 04:12:23 pm »
Quote from: Huki on April 08, 2015, 05:10:15 pm
Quote from: ollydbg on April 08, 2015, 03:38:50 pm
What was the problem if we don't have the above patch applied under Linux? Will wxFileName::FileExists() function call fail? If this does fail, then I believe this is a very long time bug. I don't have a Linux system at hand, so I can't verify myself. :)
Yes, the wxFileName::FileExists() call fails. I just tried skipping this check and keep the backward slash to see what happens: then the wxExecute does not report a failure, but no defines are collected (the defines list is empty). So it seems the wxExecute actually fails but it doesn't get reported.
By using forward slash for "/bin/" the defines are collected properly.

Thanks, and it is in trunk now.
Logged
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.
Pages: [1]   Go Up
« previous next »