After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 609552 - Gio::MemoryInputStream::add_data() mangles raw binary input
Gio::MemoryInputStream::add_data() mangles raw binary input
Status: RESOLVED FIXED
Product: glibmm
Classification: Bindings
Component: io
2.23.x
Other All
: Normal normal
: ---
Assigned To: gtkmm-forge
gtkmm-forge
Depends on:
Blocks:
 
 
Reported: 2010-02-10 16:30 UTC by Holger Seelig
Modified: 2010-03-23 17:18 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Holger Seelig 2010-02-10 16:30:19 UTC
loading a png image with MemoryInputStream and Gdk::Pixbuf::create_from_stream does not work
i get the following error:
Schwerer Fehler beim Lesen einer PNG-Bilddatei: [00][00][00][00]: invalid chunk type
(Fatal error reading PNG image file)

in comparison to that  Gdk::Pixbuf::create_from_file(tmpfile) works perfectly but this is not my intention

do i make something wrong or is this a bug

here is the code snipped from my program
the commented out lines doesn't work for me 

// ...
		std::string data;
		if (getBrowser()->getFile((*iter).getValue(), data, loadedURL))
		{
/* loading the png with MemoryInputStream does not work */
//			Glib::RefPtr<Gio::MemoryInputStream> stream = Gio::MemoryInputStream::create();
//			stream->add_data(data.c_str(), data.length());
//
//			try {
//				image = Gdk::Pixbuf::create_from_stream(stream);
//				if (image)
//					break;
//			}
//			catch(const Glib::Error& error)
//			{
//				std::cout << error.what() << std::endl;	
//			}

/* this works perfectly */
			char* tmpfile = tmpnam(NULL);
		
			std::ofstream file;
			file.open (tmpfile);
			file << data;
			file.close();

			try {
				image = Gdk::Pixbuf::create_from_file(tmpfile);
				unlink(tmpfile);
				if (image)
					break;
			}
			catch(const Glib::Error& error)
			{
				unlink(tmpfile);
				//std::cout << error.what() << std::endl;	
			}
// ...

best regards
Holger Seelig
Comment 1 Alexey Kosilin 2010-02-14 06:48:00 UTC
Iv'e just investigated exactly the same problem in my code. 
Definetely, this is the bug in Gio::MemortyInputStream::add_data implementation.
Please look at source gtkmm file memoryinputstream.ccg. It's obvious, that the implementation of 
   Gio::MemortyInputStream::add_data (const void* data, gssize len)
suppose to work with nulll-terminated strings because of using g_strdup and g_strndup to make data copy! I think, that in one of two cases g_alloc/memcpy should be used.

My workaround of the bug is:

      Glib::RefPtr<Gio::MemoryInputStream> pMis = Gio::MemoryInputStream::create() ;

      void *pDataCopy = g_malloc (sData.size()) ;
      memcpy (pDataCopy, &*sData.begin(), sData.size()) ;
      g_memory_input_stream_add_data (pMis->gobj(), pDataCopy, sData.size(), g_free) ;

      pIcon = Gdk::Pixbuf::create_from_stream (pMis) ;
Comment 2 Daniel Elstner 2010-02-14 20:02:26 UTC
You are right, g_strndup() should not be used for arbitrary binary data since it will stop at the first NUL byte and fill in the remainder with NUL characters. The g_memdup() function should be used instead.

Apart from that, I think the implicit copying for "convenience" is a misguided deviation from the original GLib C API. I'm going to file a separate bug about that.
Comment 3 Murray Cumming 2010-03-08 15:17:11 UTC
Does someone have a patch to fix this issue?
Comment 4 Murray Cumming 2010-03-21 12:34:35 UTC
Someone?
Comment 5 Alexey Kosilin 2010-03-21 13:17:37 UTC
(In reply to comment #4)
> Someone?

I really don't understand why this tiny (but annoying) bug is not still corrected...

Nevertheless, the patch for ./gio/src/memoryinputstream.ccg (glibmm-2.23.3) follows. May be it can help somebody to correct the bug. As for me, i'm not gtkmm developer, so it would be very strange and also diffcult for me if i will correct the one by myself. :)

--- memoryinputstream.ccg.old 
+++ memoryinputstream.ccg 
@@ -37,7 +37,7 @@
   if (len < 0)
     data_copy = g_strdup (static_cast<const gchar*>(data));
   else
-    data_copy = g_strndup (static_cast<const gchar*>(data), len);
+    data_copy = static_cast<gchar*> (g_memdup (data, len));
 
   g_memory_input_stream_add_data(gobj(), data_copy, len, g_free);
 }
Comment 6 Murray Cumming 2010-03-23 17:18:12 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > Someone?
> 
> I really don't understand why this tiny (but annoying) bug is not still
> corrected...

Because nobody who used this code submitted a patch until now.

> --- memoryinputstream.ccg.old 
> +++ memoryinputstream.ccg 
> @@ -37,7 +37,7 @@
>    if (len < 0)
>      data_copy = g_strdup (static_cast<const gchar*>(data));
>    else
> -    data_copy = g_strndup (static_cast<const gchar*>(data), len);
> +    data_copy = static_cast<gchar*> (g_memdup (data, len));
> 
>    g_memory_input_stream_add_data(gobj(), data_copy, len, g_free);
>  }

That looks fine. Committed to git master.

In future, please attach a git patch, and patch the ChangeLog. Thanks.