Hi Pino Toscano, Thank you for your valuable suggestions, I will make those changes and send the patch again. Thanks & Regards A Srinivas <div class="gmail_quote">On Fri, Jun 11, 2010 at 12:30 AM, <<a href="mailto:poppler-request@lists.freedesktop.org">poppler-request@lists.freedesktop.org</a>> wrote: <blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">Send poppler mailing list submissions to <a href="mailto:poppler@lists.freedesktop.org">poppler@lists.freedesktop.org</a> To subscribe or unsubscribe via the World Wide Web, visit <a href="http://lists.freedesktop.org/mailman/listinfo/poppler" target="_blank">http://lists.freedesktop.org/mailman/listinfo/poppler</a> or, via email, send a message with subject or body 'help' to <a href="mailto:poppler-request@lists.freedesktop.org">poppler-request@lists.freedesktop.org</a> You can reach the person managing the list at <a href="mailto:poppler-owner@lists.freedesktop.org">poppler-owner@lists.freedesktop.org</a> 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 <<a href="mailto:srinivas.adicherla@gmail.com">srinivas.adicherla@gmail.com</a>> Subject: [poppler] Patch to Get/Set PDF ID in the trailer dictionary, get page labels . To: <a href="mailto:poppler@lists.freedesktop.org">poppler@lists.freedesktop.org</a> Message-ID: <<a href="mailto:AANLkTincvceHMRDnHvY0yS70BAqqKOZG-GJZgIM1hrnm@mail.gmail.com">AANLkTincvceHMRDnHvY0yS70BAqqKOZG-GJZgIM1hrnm@mail.gmail.com</a>> 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: <<a href="http://lists.freedesktop.org/archives/poppler/attachments/20100610/61154402/attachment.htm" target="_blank">http://lists.freedesktop.org/archives/poppler/attachments/20100610/61154402/attachment.htm</a>> -------------- next part -------------- A non-text attachment was scrubbed... Name: srinivas_poppler.patch Type: text/x-patch Size: 4859 bytes Desc: not available URL: <<a href="http://lists.freedesktop.org/archives/poppler/attachments/20100610/61154402/attachment-0001.bin" target="_blank">http://lists.freedesktop.org/archives/poppler/attachments/20100610/61154402/attachment-0001.bin</a>> ------------------------------ Message: 2 Date: Thu, 10 Jun 2010 13:01:58 +0200 From: Pino Toscano <<a href="mailto:pino@kde.org">pino@kde.org</a>> Subject: Re: [poppler] Patch to Get/Set PDF ID in the trailer dictionary, get page labels . To: <a href="mailto:poppler@lists.freedesktop.org">poppler@lists.freedesktop.org</a> Message-ID: <<a href="mailto:201006101302.07643.pino@kde.org">201006101302.07643.pino@kde.org</a>> 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: <<a href="http://lists.freedesktop.org/archives/poppler/attachments/20100610/36cd404b/attachment-0001.pgp" target="_blank">http://lists.freedesktop.org/archives/poppler/attachments/20100610/36cd404b/attachment-0001.pgp</a>> ------------------------------ _______________________________________________ poppler mailing list <a href="mailto:poppler@lists.freedesktop.org">poppler@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/poppler" target="_blank">http://lists.freedesktop.org/mailman/listinfo/poppler</a> End of poppler Digest, Vol 64, Issue 7 ************************************** </blockquote></div>