[Podofo-users] Fwd: Re: [PATCH 05/13] PdfPagesTree::InsertPage: Fix "atIndex" is really a 0-based index

[Matthew Brincke]

Hello zyx, hello Francesco, hello all,

this is a resend of the email content (with main headers) below.
I'm sorry that e-mail didn't reach (all of) you originally.

Best regards, mabri

-- Original Message --
From: Matthew Brincke 
To: Francesco Pretto 
Cc: Dominik Seichter via Podofo-users 
Date: 25 February 2018 at 23:59
Subject: Re: [Podofo-users] [PATCH 05/13] PdfPagesTree::InsertPage: Fix 
"atIndex" is really a 0-based index
Hello Francesco, hello all,
> On 23 February 2018 at 22:25 Francesco Pretto  wrote:
> 
> 
> On 23 February 2018 at 19:27, Francesco Pretto  wrote:
> > I tested it with following code:
> >
>

I'm sorry I couldn't make it yesterday, I was too tired because I couldn't
sleep well for long enough that night. Now I've done the review and test:
patch looks good, compiles without warnings, avoids a crash with your test
code below (with a minor change, please see there), so I've accepted and
committed it in svn r1898: https://sourceforge.net/p/podofo/code/1898/

> Oops...a missing fragment in the test code. Here it again complete:
> 
> PdfMemDocument document;
> PdfRect rect;
> 
> auto pageA = document.InsertPage(rect, 0);
> auto pageB = document.InsertPage(rect, 0);
> auto pageC = document.InsertPage(rect, 2);
> 
> // PageNumber is 1-based index
> assert(pageA->GetPageNumber() == 2);
> assert(pageB->GetPageNumber() == 1);
> assert(pageC->GetPageNumber() == 3);
> 
> // Insert ouf of bounds
> auto page = document.InsertPage(rect, -1);
> assert(page->GetPageNumber() == 1);
> int pageCount = document.GetPageCount();
This definition I've changed to "unsigned int" because ... 
> page = document.InsertPage(rect, pageCount + 1);
> assert(page->GetPageNumber() == pageCount + 1);
for this I received a warning from GCC about unsigned/signed comparison.
> 
> // Insert in the middle
> page = document.InsertPage(rect, 2);
> assert(page->GetPageNumber() == 3);

Without the patch, this had GCC Adress Sanitizer report a SEGV crash
by illegal memory access (at address 0, so a null pointer dereference),
with the patch all went well. Thanks for the patch and the test code!

Best regards, mabri

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Podofo-users mailing list
Podofo-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/podofo-users

Re: [Podofo-users] Abolition of old-style casts, dynamic_cast intro request

[zyx]

On Tue, 2018-02-27 at 22:22 +0100, Matthew Brincke wrote:
> Thanks in advance for granting my request.

Hi,
from my point of view, and I confess I code in C more than in C++ these
days/months/years, I do not care much as long as the code does what it
is supposed to do. Of course, I prefer dynamic_cast when it comes to
inheritance, thus the code can verify that the passed-in object is what
it is supposed to be.

Anyway, definitely do not do any such change just before the release.
That's the worst idea one might have. There are plenty of patches to be
reviewed and, as also Francesco said, more important work to be done
first. You claimed you'll review patches during the last weekend (in
message from Fri, 23 Feb 2018), but I didn't notice anything in this
regard. Maybe stay on the safe side, do not promise things you might
not accomplish for sure. :)
Bye,
zyx

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Podofo-users mailing list
Podofo-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/podofo-users