david 69e1295384 Change the way ScriptResult::get_id and ScriptResult::get_output work to avoid ...

referencing deallocated memory.

The class was defined basically as follows:

class ScriptResult
{
private:
	std::string output;
public:
	std::string get_output() const
	{
		return this->output;
	}
};

The problem was when it was used like this, as in our script output
routines:

const char *s = sr.get_output().c_str();
printf("%s\n", s);

The reason is that the temporary std::string returned by get_output goes
out of scope after the line containing it, which invalidates the memory
pointed to by c_str(). By the time of the printf, s may be pointing to
deallocated memory.

This could have been fixed by returning a const reference that would
remain valid as long as the ScriptResult's output member is valid:

	const std::string& get_output() const
	{
		return this->output;
	}

However I noticed that get_output() was always immediately followed by a
c_str(), so I just had get_output return that instead, which has the
same period of validity.

This problem became visiable when compiling with Visual C++ 2010. The
first four bytes of script output in normal output would be garbage
(probably some kind of free list pointer). It didn't happen in XML
output, because the get_output-returned string happened to remain in
scope during that.

2010-11-09 19:47:18 +00:00

1.4 KiB

Raw Blame History

View Raw