Print Page - Crash fix in compilergcc.cpp

Developer forums (C::B DEVELOPMENT STRICTLY!) => Development => Topic started by: boaz on March 05, 2006, 08:05:49 pm

Title: Crash fix in compilergcc.cpp
Post by: boaz on March 05, 2006, 08:05:49 pm

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)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Title: Re: Crash fix in compilergcc.cpp
Post by: Michael on March 05, 2006, 08:12:25 pm

Quote from: boaz on March 05, 2006, 08:05:49 pm

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 (http://developer.berlios.de/patch/?func=addpatch&group_id=5358). The patch file generated by TortoiseSVN is good (I use it :)).

Best wishes,
Michael

Title: Re: Crash fix in compilergcc.cpp
Post by: sethjackson on March 05, 2006, 08:17:42 pm

No the patch file is messed up it uses hardcoded paths. :P 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)

Title: Re: Crash fix in compilergcc.cpp
Post by: thomas on March 05, 2006, 08:23:49 pm

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?

Title: Re: Crash fix in compilergcc.cpp
Post by: thomas on March 05, 2006, 08:49:12 pm

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 :)

Title: Re: Crash fix in compilergcc.cpp
Post by: boaz on March 05, 2006, 08:55:08 pm

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

Title: Re: Crash fix in compilergcc.cpp
Post by: boaz on March 05, 2006, 09:20:19 pm

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?

Code::Blocks Forums

Developer forums (C::B DEVELOPMENT STRICTLY!) => Development => Topic started by: boaz on March 05, 2006, 08:05:49 pm