Hello again, I couldn't help myself and created yet another patch ^^. Apart from comments, this patch does only change a single line: It moves the 'pObj->GetArray().erase( it );' call out of the while loop to the end of the function, to keep 'ref' valid until it can be safely erased from the array. This approach is possible, since the while loop is canceled anyway in this case, so the iterator remains unchanged and can be used for erase at the end. We could also remove the bFound bool and perform its check via 'it == pObj->GetArray().end()', but I left that out of the patch. Maybe my comments are a bit redundant and could be shortened.
Greetings F.E. Am Di., 9. Apr. 2019 um 08:09 Uhr schrieb F. E. <exler7...@gmail.com>: > Hello again, > > IIRC the object wasn't deleted to allow it to be reused, but I'm not sure >> if that >> was really correct (the docs aren't clear about this). >> > Unlikely. IMO the object was clearly meant to be removed, but since the > used PdfReference is destroyed in the process, > that never applied. See the last two lines of the function, it is even > stated in the comment: > >> // delete the PdfObject in the file >> delete this->GetObject()->GetOwner()->RemoveObject( ref ); >> > > Conclusion: Please wait for my acceptance of your first patch with some >> cosmetic >> and comment changes (I first need to test it), I reject v2 and v3 >> herewith. >> > All right. I understand the reasoning behind rejecting v2 and v3. I would > like to see v2 being implemented for v0.9.7, > but I fear the info will be forgotten until then ^^. > > Greetings, > F.E. > > Am Sa., 6. Apr. 2019 um 20:47 Uhr schrieb Matthew Brincke < > ma...@mailbox.org>: > >> Hello F.E., hello all, >> > On 03 April 2019 at 12:11 "F. E." <exler7...@gmail.com> wrote: >> > >> > Hello again, >> > I created two alternative patches for this issue, since the first one >> feels >> > rather ugly. With patch v2, I switched the PdfReference parameter to >> > call-by-value instead of call-by-reference. >> >> I'd like to refrain from changing API before 0.9.7 unless it's really >> necessary >> to fix a bug, here it isn't (see your first patch, which I'm going to >> accept >> after some testing going OK, from reading it it's fine). >> > With patch v3, I kept the parameter as it is, but passed a local copy >> of the >> > required PdfReference to the function. >> >> That looks clean (and the comment I'd like to put into the first one) but >> there's >> one problem: The method called is public so all users would need to >> change their >> calls to fix the problem, so that's clearly impractical (+ docs'd need to >> change). >> >> > With either of the three patches, the annotations are deleted without >> leftover >> > objects, as it should be. >> >> IIRC the object wasn't deleted to allow it to be reused, but I'm not sure >> if that >> was really correct (the docs aren't clear about this). >> >> Conclusion: Please wait for my acceptance of your first patch with some >> cosmetic >> and comment changes (I first need to test it), I reject v2 and v3 >> herewith. >> >> > Greetings, F.E. >> > >> >> Best regards, mabri >> >> >> _______________________________________________ >> Podofo-users mailing list >> Podofo-users@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/podofo-users >> >
pdfpage_delete_annotation_fix_4.patch
Description: Binary data
_______________________________________________ Podofo-users mailing list Podofo-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/podofo-users
