[poppler] poppler/Array.cc poppler/Dict.cc

[Adam Reichold]

Hello again,

sorry for the rapid succession of patches.

The attached version fixes a linker issue due to non-exported weak
symbols and fixes all users of Dict::add to not call copyString and leak
memory since std::string will take care of this including SBO.

Best regards, Adam.

P.S.: Will try to do some performance comparisons against master but
this will take at least a day due to the limitations of my machine.

Am 08.05.2018 um 08:23 schrieb Adam Reichold:
> Also cleaned up the includes a bit...
> 
> Am 08.05.2018 um 08:01 schrieb Adam Reichold:
>> Hello again again,
>>
>> Am 08.05.2018 um 07:37 schrieb Adam Reichold:
>>> Hello again,
>>>
>>> Am 08.05.2018 um 00:12 schrieb Albert Astals Cid:
>>>> El dilluns, 7 de maig de 2018, a les 23:03:00 CEST, Adam Reichold va escriure:
>>>>> Hello,
>>>>>
>>>>> it is probably already too much purely technical code churn for this
>>>>> release cycle, but the use of memmove got me thinking how an
>>>>> implementation of Dict with more STL usage would look like.
>>>>>
>>>>> The result is attached, requesting comments on whether this is
>>>>> considered useful or not.
>>>>
>>>> The remove implementation is wrong since it breaks sorting.
>>>
>>> Could you explain in more detail? My thinking was that any sublist of a
>>> sorted list is also sorted and hence removing an element of a list
>>> cannot break its sorting? (And since we do not increase the size by
>>> removing an element, we cannot reach SORT_LENGTH_LOWER_LIMIT if we were
>>> not there already.)
>>
>> Sorry, that took a minute... Because of the swap... But actually, there
>> already is std::vector::erase which does all the tricks like using
>> memmove if it is statically safe, so it should actually be simpler.
>>
>> Fixed version attached...
>>
>>> Best regards,
>>> Adam
>>>
>>>> Cheers,
>>>>   Albert
>>>>
>>>>>
>>>>> Best regards,
>>>>> Adam
>>>>>
>>>>> Am 07.05.2018 um 19:14 schrieb Albert Astals Cid:
>>>>>>  poppler/Array.cc |    2 +-
>>>>>>  poppler/Dict.cc  |    2 +-
>>>>>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> New commits:
>>>>>> commit 07b8f838b3d8859a3ad34a3140bb24475bd6ac2c
>>>>>> Author: Albert Astals Cid <aacid at kde.org>
>>>>>> Date:   Mon May 7 19:13:07 2018 +0200
>>>>>>
>>>>>>     cast to void * to bypass new gcc -Wclass-memaccess warning
>>>>>>     
>>>>>>     Yes what we're doing there is a bit nasty but it works :D
>>>>>>
>>>>>> diff --git a/poppler/Array.cc b/poppler/Array.cc
>>>>>> index 3276349f..e8aa34ea 100644
>>>>>> --- a/poppler/Array.cc
>>>>>> +++ b/poppler/Array.cc
>>>>>> @@ -112,7 +112,7 @@ void Array::remove(int i) {
>>>>>>
>>>>>>  #endif
>>>>>>  
>>>>>>    }
>>>>>>    --length;
>>>>>>
>>>>>> -  memmove( elems + i, elems + i + 1, sizeof(elems[0]) * (length - i) );
>>>>>> +  memmove( static_cast<void*>(elems + i), elems + i + 1, sizeof(elems[0])
>>>>>> * (length - i) );> 
>>>>>>  }
>>>>>>  
>>>>>>  Object Array::get(int i, int recursion) const {
>>>>>>
>>>>>> diff --git a/poppler/Dict.cc b/poppler/Dict.cc
>>>>>> index a431f7eb..bc86fd77 100644
>>>>>> --- a/poppler/Dict.cc
>>>>>> +++ b/poppler/Dict.cc
>>>>>> @@ -201,7 +201,7 @@ void Dict::remove(const char *key) {
>>>>>>
>>>>>>        gfree(entries[pos].key);
>>>>>>        entries[pos].val.free();
>>>>>>        if (pos != length) {
>>>>>>
>>>>>> -        memmove(&entries[pos], &entries[pos + 1], (length - pos) *
>>>>>> sizeof(DictEntry)); +        memmove(static_cast<void*>(&entries[pos]),
>>>>>> &entries[pos + 1], (length - pos) * sizeof(DictEntry));> 
>>>>>>        }
>>>>>>      
>>>>>>      }
>>>>>>    
>>>>>>    } else {
>>>>>>
>>>>>> _______________________________________________
>>>>>> poppler mailing list
>>>>>> poppler at lists.freedesktop.org
>>>>>> https://lists.freedesktop.org/mailman/listinfo/poppler
>>>>
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> poppler mailing list
>>>> poppler at lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/poppler
>>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> poppler mailing list
>>> poppler at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/poppler
>>>
>>>
>>>
>>> _______________________________________________
>>> poppler mailing list
>>> poppler at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/poppler
>>>
>>>
>>> _______________________________________________
>>> poppler mailing list
>>> poppler at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/poppler
-------------- next part --------------
A non-text attachment was scrubbed...
Name: simplify-dict-v4.diff
Type: text/x-patch
Size: 24365 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/poppler/attachments/20180508/5683e5e0/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/poppler/attachments/20180508/5683e5e0/attachment-0001.sig>