[poppler] poppler Digest, Vol 64, Issue 7
srinivas adicherla
srinivas.adicherla at gmail.com
Thu Jun 10 22:04:23 PDT 2010
Hi Pino Toscano,
Thank you for your valuable suggestions, I will make those changes and
send the patch again.
Thanks & Regards
A Srinivas
On Fri, Jun 11, 2010 at 12:30 AM, <poppler-request at lists.freedesktop.org>wrote:
> Send poppler mailing list submissions to
> poppler at lists.freedesktop.org
>
> To subscribe or unsubscribe via the World Wide Web, visit
> http://lists.freedesktop.org/mailman/listinfo/poppler
> or, via email, send a message with subject or body 'help' to
> poppler-request at lists.freedesktop.org
>
> You can reach the person managing the list at
> poppler-owner at lists.freedesktop.org
>
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of poppler digest..."
>
>
> Today's Topics:
>
> 1. Patch to Get/Set PDF ID in the trailer dictionary, get page
> labels . (srinivas adicherla)
> 2. Re: Patch to Get/Set PDF ID in the trailer dictionary, get
> page labels . (Pino Toscano)
>
>
> ----------------------------------------------------------------------
>
> Message: 1
> Date: Thu, 10 Jun 2010 15:43:25 +0530
> From: srinivas adicherla <srinivas.adicherla at gmail.com>
> Subject: [poppler] Patch to Get/Set PDF ID in the trailer dictionary,
> get page labels .
> To: poppler at lists.freedesktop.org
> Message-ID:
> <AANLkTincvceHMRDnHvY0yS70BAqqKOZG-GJZgIM1hrnm at mail.gmail.com>
> Content-Type: text/plain; charset="iso-8859-1"
>
> Hi,
>
> This Patch does the following:
>
> 1) gets and set the pdf IDs in the trailer dictionary.
> 2) get all page labels & get page labels by index.
>
> Kindly let me know if You commit this patch.
>
>
> Thanks
> A Srinivas
> -------------- next part --------------
> An HTML attachment was scrubbed...
> URL: <
> http://lists.freedesktop.org/archives/poppler/attachments/20100610/61154402/attachment.htm
> >
> -------------- next part --------------
> A non-text attachment was scrubbed...
> Name: srinivas_poppler.patch
> Type: text/x-patch
> Size: 4859 bytes
> Desc: not available
> URL: <
> http://lists.freedesktop.org/archives/poppler/attachments/20100610/61154402/attachment-0001.bin
> >
>
> ------------------------------
>
> Message: 2
> Date: Thu, 10 Jun 2010 13:01:58 +0200
> From: Pino Toscano <pino at kde.org>
> Subject: Re: [poppler] Patch to Get/Set PDF ID in the trailer
> dictionary, get page labels .
> To: poppler at lists.freedesktop.org
> Message-ID: <201006101302.07643.pino at kde.org>
> Content-Type: text/plain; charset="iso-8859-1"
>
> Hi,
>
> I'm not a poppler-glib developer, but there are few notes below.
>
> Alle gioved? 10 giugno 2010, srinivas adicherla ha scritto:
> > +/* following three functions has added to get and set pdf ID stored
> > in Trailer Dictionary*/
> > +static void
> > +get_id(char* id,char *buff) {
> > +
> > + int i;
> > + for( i = 0; i < 16; i++ ) {
> > +
> > + char *buf = (char*)malloc(3*sizeof(char));
> > + sprintf(buf,"%02x",id[i] & 0xff);
> > +
> > + if(i == 0) strcpy(buff,buf);
> > + else strcat(buff,buf);
> > +
> > + free (buf);
> > + }
> > +}
>
> This is pretty inefficient: you are allocating on the heap 3 bytes 16
> times (!). It would be much better if you could just use an array of 3
> chars on stack, and reuse it at every iteration.
> (Minor: a better naming than 'buf' and 'buff' would help a lot to
> distinguish them.)
>
> > +
> > +gboolean
> > +poppler_document_get_trailer_id(char *uri,char *password,char
> > *id[2]) {
> > [...]
> > + for(int i=0; i<2; i++) {
> > + id[i] = (char*)malloc(32*sizeof(char));
> > + }
>
> Excluding the fact that such for loop is useless (just manually do the
> two calls it does), why do you dynamically malloc the two result strings
> if you already know they will be 32 chars long? Then, instead of a out
> parameter of a two-strings array, just use two out parameters of 32
> bytes.
>
> > +void *
> > +poppler_document_generate_and_set_id(char *uri,char *password) {
>
> If this function returns nothing, then declare it as void, not as void*.
>
> > +char*
> > +poppler_document_get_page_label_by_index(PopplerDocument *document,
> > + int index)
> > +{
> > + GooString *string = new GooString();
> > + Catalog *catalog = document->doc->getCatalog ();
> > +
> > + catalog->indexToLabel(index,string);
> > +
> > + char *label = string->getCString();
> > + return label;
> > +}
>
> 'string' is leaked here, just put it on stack.
>
> > +char**
> > +poppler_document_get_page_labels(PopplerDocument *document)
> > +{
> > +
> > + char **labels;
> > + Catalog *catalog= document->doc->getCatalog ();
> > + int npages = poppler_document_get_n_pages(document);
> > +
> > + labels = (char**)malloc(sizeof(char*)*npages);
> > +
> > + int i;
> > + for(i = 0; i < npages; i++) {
> > +
> > + GooString *label = new GooString();
> > + catalog->indexToLabel(i,label);
> > + labels[i] = label->getCString();
> > + }
> > + return labels;
> > +}
>
> This is tricker: the 'label' allocated at each loop is leaked, but you
> use its internal storage as return value. As before, put the GooString
> on stack, but strdup its storage.
>
> > +/* Labels */
> > +char*
> > +poppler_document_get_page_label_by_index(PopplerDocument *document,
> > + int index);
> > +char**
> > +poppler_document_get_page_labels(PopplerDocument *document);
>
> A label is specific to every page, so I guess these two functions would
> quite more suited as part of the PopplerPage API.
> Also, they miss they are not documented with gtk-doc, just like the rest
> of the glib API.
>
> > +/*Get the PDF ID from the trailer dictionary. Pdf Id contains two
> > strings of length 16 bytes each, represented in Hex*/
> > +gboolean poppler_document_get_trailer_id(char *uri,char
> > *password,char *id[2]);
> > +
> > +/*Generate the Pdf Id and set it in the Pdf file*/
> > +void *poppler_document_generate_and_set_id(char *uri,char
> > *password);
>
> Most probably they should not be just "free functions", but be part of
> the PopplerDocument API, so you first open a document as usual, then get
> its trailer and then free the document.
> Also, the uri and password parameters should be 'const', to indicate
> they won't be changed in the functions. And as above, they miss gtk-doc
> API documentation.
>
> --
> Pino Toscano
> -------------- next part --------------
> A non-text attachment was scrubbed...
> Name: not available
> Type: application/pgp-signature
> Size: 190 bytes
> Desc: This is a digitally signed message part.
> URL: <
> http://lists.freedesktop.org/archives/poppler/attachments/20100610/36cd404b/attachment-0001.pgp
> >
>
> ------------------------------
>
> _______________________________________________
> poppler mailing list
> poppler at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/poppler
>
>
> End of poppler Digest, Vol 64, Issue 7
> **************************************
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/poppler/attachments/20100611/1e6cf22a/attachment.htm>
More information about the poppler
mailing list