On Fri, 18 Jan 2019 at 01:14, Matthew Brincke <ma...@mailbox.org> wrote:
>
> Hello Francesco, hello all,
>
> > On 14 January 2019 at 23:08 Francesco Pretto <cez...@gmail.com> wrote:
> >
> >
> > From: Francesco Pretto <cez...@gmail.com>
> > ---src/base/PdfDictionary.cpp | 17 ++++++++---------
> > 1 file changed, 8 insertions(+), 9 deletions(-)
>
> the lines your patch removes each contain an explicit or
> implicit lookup, whereas your proposal only has one lookup
> per method (class members are not called "functions" IIRC)
> so it removes 5 look-ups (therefore I've changed the subject).

Thank you! Yes, it's 5 lookups removed, the code was very inefficient.

> More importantly, this patch seems to depend on [PATCH 4/5] in
> this series for correctness (to avoid a memory leak) because
> it removes a "delete" operator invocation without replacement
> (IIRC: doesn't std::map::erase(), like std::vector::erase(),
> only remove the item(s) from the container but doesn't free
> their memory, for "reference" types, i.e. not primitives?).
>

Are you talking about RemoveKey() method, right? I think the delete
invocation is still missing, also in [PATCH 4/5], as map::erase()
can't release memory for pointers.

Fixed method would be:

    TKeyMap::iterator found = m_mapKeys.find( identifier );
    if( found != m_mapKeys.end() )
    {
        AssertMutable();
        delete found->second;
        m_mapKeys.erase( found );
        m_bDirty = true;
        return true;
    }

I'm a bit in a stale situation now. Can I fix the change in a bigger
merged patch 1-5?


_______________________________________________
Podofo-users mailing list
Podofo-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/podofo-users

Reply via email to