Re: [Podofo-users] [PATCH 2/5] PdfDictionary: Removed 5 unneeded lookups in AddKey and RemoveKey methods

[Francesco Pretto]

Hello Matthew and all,

On Fri, 18 Jan 2019 at 15:06, Matthew Brincke <ma...@mailbox.org> wrote:
>
> > 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;
> >     }
>
> Thank you for providing the fix here, I'll credit you for the
> patch and the fix because I'd like to accept the first two
> patches in this series today (commit them after testing with
> different compilers on GNU/Linux).
>
>[...other thread...]
> The [PATCH 2/5] looks fine, but I'd like to put it on hold for
> a bit because the location it's to be applied to is implicated
> in issue #39 [2] and I plan to analyze and fix that first.

Now that you pushed change for #39, and that [PATCH 2/5] should have
been already reviewed by you (with additional fix above, attached updated
patch), are you going to merge the change? I would like to create branch
for PdfObject ownership based on this.

Thank you,
Francesco

commit 8a84f1347619f9c3c09a8866d81237f53301b0e5
Author: Francesco Pretto <francesco.pretto@euronovate.com>
Date:   Fri Jan 18 12:08:20 2019 +0100

    PdfDictionary: Removed 2 unneeed lookups in AddKey and RemoveKey functions

diff --git a/src/base/PdfDictionary.cpp b/src/base/PdfDictionary.cpp
index bf2623c..0693877 100644
--- a/src/base/PdfDictionary.cpp
+++ b/src/base/PdfDictionary.cpp
@@ -139,14 +139,14 @@ void PdfDictionary::AddKey( const PdfName & identifier, const PdfObject & rObjec
         PODOFO_RAISE_ERROR( ePdfError_InvalidDataType );
     }
     */
-
-    if( m_mapKeys.find( identifier ) != m_mapKeys.end() )
+    PdfObject *objToInsert = new PdfObject(rObject);
+    std::pair<TKeyMap::iterator, bool> inserted = m_mapKeys.insert( std::make_pair( identifier, objToInsert ) );
+    if ( !inserted.second )
     {
-        delete m_mapKeys[identifier];
-        m_mapKeys.erase( identifier );
+        delete inserted.first->second;
+        inserted.first->second = objToInsert;
     }
 
-	m_mapKeys[identifier] = new PdfObject( rObject );
     m_bDirty = true;
 }
 
@@ -246,12 +246,12 @@ bool PdfDictionary::HasKey( const PdfName & key ) const
 
 bool PdfDictionary::RemoveKey( const PdfName & identifier )
 {
-    if( HasKey( identifier ) )
+    TKeyMap::iterator found = m_mapKeys.find( identifier );
+    if( found != m_mapKeys.end() )
     {
         AssertMutable();
-        delete m_mapKeys[identifier];
-
-        m_mapKeys.erase( identifier );
+        delete found->second;
+        m_mapKeys.erase( found );
         m_bDirty = true;
         return true;
     }


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