[Poppler-bugs] [Bug 36653] Missing functions for setting document's properties

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=36653

--- Comment #46 from Carlos Garcia Campos <carlosgc at gnome.org> ---
Comment on attachment 124573
  --> https://bugs.freedesktop.org/attachment.cgi?id=124573
[PATCH 2/7] Added DocInfo setters & getters

Review of attachment 124573:
-----------------------------------------------------------------

Looks good to me, I have only a couple of comments, but to not delay this more
I'll push a modified patch with my comments addressed.

::: poppler/PDFDoc.cc
@@ +655,5 @@
> +
> +  GooString *result;
> +
> +  if (entryObj.isString()) {
> +    result = new GooString(entryObj.getString());

As I suggested in a previous review, we can save the allocation here, by using
GooString::takeString().

::: poppler/PDFDoc.h
@@ +234,5 @@
>  
> +  // Create and return the document's Info dictionary if none exists.
> +  // Otherwise return the existing one.
> +  Object *createDocInfoIfNoneExists(Object *obj) { return xref->createDocInfoIfNoneExists(obj); }
> +  Object *createDocInfoIfNoneExistsNF(Object *obj) { return xref->createDocInfoIfNoneExistsNF(obj); }

This doesn't seem to be used anywhere, I prefer to not add dead code, if we
eventually need this we can just add it.

::: poppler/XRef.cc
@@ +1312,5 @@
> +
> +  return obj;
> +}
> +
> +Object *XRef::createDocInfoIfNoneExistsNF(Object *obj) {

Ditto.

@@ +1338,5 @@
> +}
> +
> +void XRef::removeDocInfo() {
> +  Object infoObjRef;
> +  getDocInfoNF(&infoObjRef);

Since this is public in the end, we should check that it's not null.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/poppler-bugs/attachments/20160703/c2ce0002/attachment.html>