[poppler] Second opinion for patch

[Albert Astals Cid]

El divendres, 18 de novembre de 2016, a les 23:13:23 CET, Albert Astals Cid va 
escriure:
> El divendres, 18 de novembre de 2016, a les 2:17:27 CET, William Bader va
> 
> escriure:
> > The early return seems to exit when two consecutive objects have the same
> > id and gen.
> 
> Which early return? I don't see any check for two consecutive objects with
> the same id in that code.

I ran a quick test over all the files i have around and found 3 other "broken" 
files in which with this patch we show the same that Adobe shows while the 
other files are unchanged, so i'll be commiting this.

Cheers,
  Albert

> 
> > Is that a normal marker for the end of the list or was that added as a
> > check to prevent an infinite loop on a bad file with a circular list?
> > Would it be better to replace the test with a better check for an
> > infinite loop? Applications that use poppler shouldn't hang if a user
> > opens a corrupt PDF.
> > 
> > 
> > In the example file, is the entry that triggers the early exit a duplicate
> > of the previous entry? Would it be better to keep the test but skip the
> > entry instead of exiting the loop?
> 
> No it's not a duplicate entry, as i said, it's just that the "Last" pointer
> is not pointing to the real Last if you follow the "Next" pointer of the
> siblings.
> 
> Cheers,
>   Albert
> 
> > William
> > 
> > 
> > ________________________________
> > From: poppler <poppler-bounces at lists.freedesktop.org> on behalf of Albert
> > Astals Cid <aacid at kde.org> Sent: Thursday, November 17, 2016 4:05 PM
> > To: poppler at lists.freedesktop.org
> > Subject: Re: [poppler] Second opinion for patch
> > 
> > El dijous, 17 de novembre de 2016, a les 17:48:24 CET, Carlos Garcia
> > Campos
> > va
> > 
> > escriure:
> > > Albert Astals Cid <aacid at kde.org> writes:
> > > > Can someone have a look at
> > > > https://bugs.freedesktop.org/show_bug.cgi?id=98732
> > > > 
> > > > I'd like a second opinion on the patch, it fixes the problem on that
> > > > broken
> > > > file for me but before commiting i'd like to know i'm not missing
> > > > something.
> > > 
> > > I guess the file is broken because "Last" is required field in outline
> > > dictionary. In that case I would keep using Last if present, and simply
> > > remove the early return in OutlineItem::readItemList() otherwise.
> > 
> > Yes it's broken, the file does have a "Last", it's just that the "Last"
> > doesn't really point to the last but to somehwere in the middle (if you
> > use
> > the next->next->next).
> > 
> > Given that Adobe Reader shows all of the items i guess it also ignores it
> > and just relies on next.
> > 
> > Cheers,
> > 
> >   Albert
> >   
> > > > Cheers,
> > > > 
> > > >   Albert
> > > > 
> > > > _______________________________________________
> > > > poppler mailing list
> > > > poppler at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/poppler
> > 
> > poppler Info Page -
> > freedesktop.org<https://lists.freedesktop.org/mailman/listinfo/poppler>
> > lists.freedesktop.org
> > Subscribing to poppler: Subscribe to poppler by filling out the following
> > form. You will be sent email requesting confirmation, to prevent others
> > from ...
> > 
> > 
> > 
> > 
> > 
> > _______________________________________________
> > poppler mailing list
> > poppler at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/poppler
> > poppler Info Page -
> > freedesktop.org<https://lists.freedesktop.org/mailman/listinfo/poppler>
> > lists.freedesktop.org
> > Subscribing to poppler: Subscribe to poppler by filling out the following
> > form. You will be sent email requesting confirmation, to prevent others
> > from ...
> 
> _______________________________________________
> poppler mailing list
> poppler at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/poppler