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

[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.)

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
> 

-------------- 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/8380143e/attachment.sig>