gnash-commit
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Gnash-commit] gnash ChangeLog server/button_character_instanc...


From: Sandro Santilli
Subject: Re: [Gnash-commit] gnash ChangeLog server/button_character_instanc...
Date: Wed, 25 Apr 2007 12:04:41 +0200

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;




reply via email to

[Prev in Thread] Current Thread [Next in Thread]