[poppler] unit test for GLib bindings

Kouhei Sutou kou at cozmixng.org
Sat Apr 25 21:04:00 PDT 2009


Hi,

In <1240679760.4223.30.camel at charmaleon>
  "Re: [poppler] unit test for GLib bindings" on Sat, 25 Apr 2009 19:16:00 +0200,
  Carlos Garcia Campos <carlosgc at gnome.org> wrote:

>>  }
>>  
>>  static void
>> @@ -518,7 +523,8 @@ attachment_inspect (GString *string, gconstpointer data, gpointer user_data)
>>                            attachment->size);
>>    g_string_append_printf (string, "mtime=<%d>, ", attachment->mtime);
>>    g_string_append_printf (string, "ctime=<%d>, ", attachment->ctime);
>> -  g_string_append_printf (string, "checksum=<%s>", attachment->checksum->str);
>> +  g_string_append_printf (string, "checksum=<%s>",
>> +                          attachment->checksum ? attachment->checksum->str : "NULL");
>>    g_string_append (string, ">");
>>  }
> 
> still crashes. I've noticed that the checksum string is created even
> when there isn't a checksum, I've just fixed it. 

I've followed the changes in githumb.com repository.
# It seems that the test is good test because it found a
# bug. :)

> isn't it reproducible for you?

I can't reproduce it on my environment.
But I can reproduce it on LANG=fr_FR.UTF8 environment.

I wrote a small test script to reproduce this problem
without Cutter:

diff --git a/glib/Makefile.am b/glib/Makefile.am
index 2c7ca1a..79cb423 100644
--- a/glib/Makefile.am
+++ b/glib/Makefile.am
@@ -86,7 +86,7 @@ libpoppler_glib_la_LIBADD =				\
 libpoppler_glib_la_LDFLAGS = -version-info 4:0:0 @create_shared_lib@ @auto_import_flags@
 
 if BUILD_WITH_GDK
-noinst_PROGRAMS = test-poppler-glib
+noinst_PROGRAMS = test-poppler-glib test-attachment
 
 test_poppler_glib_SOURCES =			\
        test-poppler-glib.cc
@@ -98,6 +98,17 @@ test_poppler_glib_LDADD =			\
 	$(GDK_LIBS)				\
 	$(FREETYPE_LIBS)			\
 	$(cairo_libs)
+
+test_attachment_SOURCES =			\
+       test-attachment.c
+
+test_attachment_LDADD =				\
+	$(top_builddir)/poppler/libpoppler.la	\
+	libpoppler-glib.la			\
+	$(POPPLER_GLIB_LIBS)			\
+	$(GDK_LIBS)				\
+	$(FREETYPE_LIBS)			\
+	$(cairo_libs)
 endif
 
 BUILT_SOURCES =					\
diff --git a/glib/test-attachment.c b/glib/test-attachment.c
new file mode 100644
index 0000000..137925d
--- /dev/null
+++ b/glib/test-attachment.c
@@ -0,0 +1,36 @@
+/* -*- c-file-style: "gnu" -*- */
+
+#include <poppler.h>
+
+int main (int argc, char *argv[])
+{
+  int i;
+
+  g_type_init ();
+
+  for (i = 0; i < 10; i++)
+    {
+      PopplerDocument *document;
+      GList *attachments, *node;
+      GError *error = NULL;
+
+      document = poppler_document_new_from_file (argv[1], NULL, &error);
+      if (document == NULL)
+        {
+          g_print ("%s\n", error->message);
+          return(-1);
+        }
+
+      attachments = poppler_document_get_attachments (document);
+      for (node = attachments; node; node = g_list_next (node))
+        {
+          PopplerAttachment *attachment = node->data;
+          poppler_attachment_save (attachment, "/tmp/attachment", NULL);
+        }
+      g_object_unref (document);
+      g_list_foreach (attachments, (GFunc) g_object_unref, NULL);
+      g_list_free (attachments);
+    }
+
+  return 0;
+}

PopplerAttachment should be g_object_unref()-ed before
PopplerDocument is g_object_unref()-ed. Is it right
specification?

The behavior isn't good for bindings for script language
that its GC isn't based on reference count. (e.g. Ruby)
In the script language bindings, (e.g. Ruby/Poppler),
PopplerAttachment may be destroyed before PopplerDocument if
PopplerAttachment doesn't refer PopplerDocument.

What about PopplerAttachment refers its document? It will
fix the GC related problem. (And Cutter based test will
also work. :-)

diff --git a/glib/poppler-attachment.cc b/glib/poppler-attachment.cc
index f6dbfd2..78bc72f 100644
--- a/glib/poppler-attachment.cc
+++ b/glib/poppler-attachment.cc
@@ -29,11 +29,13 @@
 typedef struct _PopplerAttachmentPrivate PopplerAttachmentPrivate;
 struct _PopplerAttachmentPrivate
 {
-  Object obj_stream;
+  Object *obj_stream;
+  PopplerDocument *document;
 };
 
 #define POPPLER_ATTACHMENT_GET_PRIVATE(obj) (G_TYPE_INSTANCE_GET_PRIVATE ((obj), POPPLER_TYPE_ATTACHMENT, PopplerAttachmentPrivate))
 
+static void poppler_attachment_dispose (GObject *obj);
 static void poppler_attachment_finalize (GObject *obj);
 
 G_DEFINE_TYPE (PopplerAttachment, poppler_attachment, G_TYPE_OBJECT)
@@ -46,11 +48,35 @@ poppler_attachment_init (PopplerAttachment *attachment)
 static void
 poppler_attachment_class_init (PopplerAttachmentClass *klass)
 {
+  G_OBJECT_CLASS (klass)->dispose = poppler_attachment_dispose;
   G_OBJECT_CLASS (klass)->finalize = poppler_attachment_finalize;
   g_type_class_add_private (klass, sizeof (PopplerAttachmentPrivate));
 }
 
 static void
+poppler_attachment_dispose (GObject *obj)
+{
+  PopplerAttachmentPrivate *priv;
+
+  priv = POPPLER_ATTACHMENT_GET_PRIVATE (obj);
+
+  if (priv->obj_stream)
+    {
+      priv->obj_stream->free();
+      delete priv->obj_stream;
+      priv->obj_stream = NULL;
+    }
+
+  if (priv->document)
+    {
+      g_object_unref (priv->document);
+      priv->document = NULL;
+    }
+
+  G_OBJECT_CLASS (poppler_attachment_parent_class)->dispose (obj);
+}
+
+static void
 poppler_attachment_finalize (GObject *obj)
 {
   PopplerAttachment *attachment;
@@ -69,8 +95,6 @@ poppler_attachment_finalize (GObject *obj)
     g_string_free (attachment->checksum, TRUE);
   attachment->checksum = NULL;
   
-  POPPLER_ATTACHMENT_GET_PRIVATE (attachment)->obj_stream.free();
-
   G_OBJECT_CLASS (poppler_attachment_parent_class)->finalize (obj);
 }
 
@@ -81,12 +105,16 @@ _poppler_attachment_new (PopplerDocument *document,
 			 EmbFile         *emb_file)
 {
   PopplerAttachment *attachment;
+  PopplerAttachmentPrivate *priv;
 
   g_assert (document != NULL);
   g_assert (emb_file != NULL);
 
   attachment = (PopplerAttachment *) g_object_new (POPPLER_TYPE_ATTACHMENT, NULL);
-  
+  priv = POPPLER_ATTACHMENT_GET_PRIVATE (attachment);
+
+  priv->document = (PopplerDocument *) g_object_ref (document);
+
   if (emb_file->name ())
     attachment->name = _poppler_goo_string_to_utf8 (emb_file->name ());
   if (emb_file->description ())
@@ -101,7 +129,8 @@ _poppler_attachment_new (PopplerDocument *document,
 	  attachment->checksum = g_string_new_len (emb_file->checksum ()->getCString (),
 						   emb_file->checksum ()->getLength ());
   
-  emb_file->streamObject().copy(&POPPLER_ATTACHMENT_GET_PRIVATE (attachment)->obj_stream);
+  priv->obj_stream = new Object();
+  emb_file->streamObject().copy(priv->obj_stream);
 
   return attachment;
 }
@@ -214,7 +243,7 @@ poppler_attachment_save_to_callback (PopplerAttachment          *attachment,
 
   g_return_val_if_fail (POPPLER_IS_ATTACHMENT (attachment), FALSE);
 
-  stream = POPPLER_ATTACHMENT_GET_PRIVATE (attachment)->obj_stream.getStream();
+  stream = POPPLER_ATTACHMENT_GET_PRIVATE (attachment)->obj_stream->getStream();
   stream->reset();
 
   do


Thanks,
--
kou


More information about the poppler mailing list