Skip to content

LPE/param/array|unit: Fix unnecessary dyn alloc & leaks

Daniel Boles requested to merge dboles/inkscape:djb_live_effect_leaks into master 
commit d7d74e973b929b6faa5203d2ac1b3ade60542732 (HEAD -> djb_live_effect_leaks)
Author: Daniel Boles <dboles.src+inkscape@gmail.com>
Date:   Thu Sep 7 12:56:20 2023 +0100

    LPE/param/unit: Fix ! delete of dyn-allocated unit
    
    If we allocate it, we should also free it. Since it might be a reference
    to the defunit, this is not as nice as it could be, but it is *better*…!
    
    Found by ASAN:
    
    ```none
    Direct leak of 432 byte(s) in 3 object(s) allocated from:
        #0 0x7fd53aad90f8 in operator new(unsigned long) ../../../../src/libsanitizer/asan/asan_new_delete.cpp:95
        #1 0x7fd537cdeba8 in Inkscape::LivePathEffect::UnitParam::param_set_value(Inkscape::Util::Unit const&) /home/daniel/src/remote/inkscape/inkscape/src/live_effects/parameter/unit.cpp:73
        #2 0x7fd537cdec3b in Inkscape::LivePathEffect::UnitParam::param_readSVGValue(char const*) /home/daniel/src/remote/inkscape/inkscape/src/live_effects/parameter/unit.cpp:39
        #3 0x7fd537943a95 in Inkscape::LivePathEffect::Effect::readallParameters(Inkscape::XML::Node const*) /home/daniel/src/remote/inkscape/inkscape/src/live_effects/effect.cpp:1709
        #4 0x7fd537944466 in Inkscape::LivePathEffect::Effect::New(Inkscape::LivePathEffect::EffectType, LivePathEffectObject*) /home/daniel/src/remote/inkscape/inkscape/src/live_effects/effect.cpp:1111
    [...]
    ```

commit 98b8ae04e6eabd242113e047783d86063d209e0d
Author: Daniel Boles <dboles.src+inkscape@gmail.com>
Date:   Thu Sep 7 12:47:49 2023 +0100

    LPE/param/array: Fix unnecessary dyn alloc & leak
    
    As found by ASAN. The new is pointless as we just copy the object then
    throw it away. Instead, just move (or copy if move not implemented) it.
    
    ```none
    Direct leak of 480 byte(s) in 12 object(s) allocated from:
        #0 0x7fd53aad90f8 in operator new(unsigned long) ../../../../src/libsanitizer/asan/asan_new_delete.cpp:95
        #1 0x7fd537c6e3d5 in Inkscape::LivePathEffect::ArrayParam<std::vector<NodeSatellite, std::allocator<NodeSatellite> > >::readsvg(char const*) /home/daniel/src/remote/inkscape/inkscape/src/live_effects/parameter/array.cpp:100
        #2 0x7fd537a76b9c in Inkscape::LivePathEffect::ArrayParam<std::vector<NodeSatellite, std::allocator<NodeSatellite> > >::param_readSVGValue(char const*) /home/daniel/src/remote/inkscape/inkscape/src/live_effects/parameter/array.h:67
        #3 0x7fd537943a95 in Inkscape::LivePathEffect::Effect::readallParameters(Inkscape::XML::Node const*) /home/daniel/src/remote/inkscape/inkscape/src/live_effects/effect.cpp:1709
        #4 0x7fd537944466 in Inkscape::LivePathEffect::Effect::New(Inkscape::LivePathEffect::EffectType, LivePathEffectObject*) /home/daniel/src/remote/inkscape/inkscape/src/live_effects/effect.cpp:1111
        #5 0x7fd537c68448 in LivePathEffectObject::set(SPAttr, char const*) /home/daniel/src/remote/inkscape/inkscape/src/live_effects/lpeobject.cpp:101
        #6 0x7fd537ebc8fa in SPObject::setKeyValue(SPAttr, char const*) /home/daniel/src/remote/inkscape/inkscape/src/object/sp-object.cpp:1107
        #7 0x7fd537ebc98a in SPObject::readAttr(SPAttr) /home/daniel/src/remote/inkscape/inkscape/src/object/sp-object.cpp:1125
        #8 0x7fd537c67c46 in LivePathEffectObject::build(SPDocument*, Inkscape::XML::Node*) /home/daniel/src/remote/inkscape/inkscape/src/live_effects/lpeobject.cpp:34
    [...]
    ```

Merge request reports