Crash fix in compilergcc.cpp

[boaz]

For some esoteric messages from GCC C:B would crash.

if the message output from GCC would contain %(something) like for example in my case:
n:\temp/cc6daaaa.s: Assembler messages:
n:\temp/cc6daaaa.s:397: Warning: translating to `fst %st(1)'

C:B would crash. This is because the Printf(...) like functions: LogToStdOut() and Log(…) where called with the message from GCC passed on the format_str parameter.

(I guess this is the oldest bug in the book, I keep doing them still). The fix also eliminates a few cycles as it does not need the wxString extra copy with the ' output + _T"\n" '.

I have only fixed these places that where in my code path, but a quick search for LogToStdOut() reviles this mistake is done in other places as well. Should I make a patch for all other places? (that I can find)

I have inlined the diff file as made by tortoiseSVN is that ok or should I send it somewhere else? other format ?

vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv

Index: M:/Dinosaur/OneSource/codeblocks/src/plugins/compilergcc/compilergcc.cpp
===================================================================
--- M:/Dinosaur/OneSource/codeblocks/src/plugins/compilergcc/compilergcc.cpp   (revision 2139)
+++ M:/Dinosaur/OneSource/codeblocks/src/plugins/compilergcc/compilergcc.cpp   (working copy)
@@ -2614,7 +2614,7 @@
 
 void CompilerGCC::AddOutputLine(const wxString& output, bool forceErrorColor)
 {
-    Manager::Get()->GetMessageManager()->LogToStdOut(output + _T('\n'));
+    Manager::Get()->GetMessageManager()->LogToStdOut(_T("%s\n") ,output.c_str() );
 
     size_t maxErrors = Manager::Get()->GetConfigManager(_T("compiler"))->ReadInt(_T("/max_reported_errors"), 50);
     if (maxErrors > 0)
@@ -2674,7 +2674,7 @@
     }
 
    if (!output.IsEmpty())
-        Manager::Get()->GetMessageManager()->Log(m_PageIndex, output.c_str());
+        Manager::Get()->GetMessageManager()->Log(m_PageIndex, _T("%s"), output.c_str());
 }
 
 void CompilerGCC::OnGCCTerminated(CodeBlocksEvent& event)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

 

 

[Michael]

I have only fixed these places that where in my code path, but a quick search for LogToStdOut() reviles this mistake is done in other places as well. Should I make a patch for all other places? (that I can find)

I have inlined the diff file as made by tortoiseSVN is that ok or should I send it somewhere else? other format ?

You should post your patch at BerliOS. See here. The patch file generated by TortoiseSVN is good (I use it .

Best wishes,
Michael

[sethjackson]

No the patch file is messed up it uses hardcoded paths. Replace the header with this.

Code: diff

Index: src/plugins/compilergcc/compilergcc.cpp
===================================================================
--- src/plugins/compilergcc/compilergcc.cpp   (revision 2139)
+++ src/plugins/compilergcc/compilergcc.cpp   (working copy)

[thomas]

Hmm... maybe we should overload MessageManager::Log with a wxString version.
That would fix the problem and would save us from searching in other places, but it would still preserve the original funcitonality... Yiannis?

[thomas]

I overloaded them like this for testing purposes:

Code

		void LogToStdOut(const wxString& msg);
		void LogToStdOut(const wxChar* msg, ...);
		void Log(const wxString& msg);
		void Log(const wxChar* msg, ...);

Compiles fine, runs fine.

Inserted int %a = %b + %c; into copystrings.cpp for testing and got this:

Code

copystrings.cpp: In constructor `copystrings::copystrings()':
copystrings.cpp:26: error: expected unqualified-id before '%token
Process terminated with status 1 (0 minutes, 4 seconds)
1 errors, 1 warnings

Guess that fixes it universally

[boaz]

Thanks I edited registered and posted at BerliOS.

What is the costume with Patches, should I also post an explanation/comments here? or Just send a patch there?

What I do not understand is where to write my changelog ? do you put that somehere inside the patch file? what is the syntax for that?

Free life
Boaz

[boaz]

Tomas's Idea is good.

Tomas: will you post your changes? Where you careful with the implementation of the new members?

What about the extra string copy of the: 'xxx + _T("\n")' maybe in this case we should stay with the first option?