On Wed, Apr 25, 2007 at 10:18:41AM +0800, zou lunkai wrote:
> +/*public*/
> +const std::string
> +character::getTarget() const
> +{
> +
> +  // TODO: maybe cache computed target?
> +
> +       std::string target_dot;
...
> +       return target_dot;
> +}

Returning const std::string makes no sense, it's no const if we return by value 
!
Same for getTargetPath().
Note that the original code was in sprite_instance, which kept a "cache", so 
returned by const ref.


> +const char*   <----------------------------------[1]
> +character::get_text_value() const
> +{
> +       return getTarget().c_str();
> +}
> +
> 
> [1] return a pointer to a local string!!! We won't get any text value
> from it.   This caused odd problem with my event related testcase(just
> changed the event order without invoking any other obvious problem),
> and cost me an hour to track this. Our testsuite seems too weak to
> prevent any thing.

Again, this worked for sprite_instance because it kept a cache of the target 
paths. If we don't keep a cache we'll have to modify get_text_value() to return 
by
string value instead (no trivial change). Alternatively we reintroduce the 
cache just for the
sake of returning bu const-ref.

> Please fix this!

Indeed. UdoG, I think it's your turn :)

--strk;


_______________________________________________
Gnash-commit mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/gnash-commit

Reply via email to