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 709793 - libsoup memory leak
libsoup memory leak
Status: RESOLVED FIXED
Product: libsoup
Classification: Core
Component: Misc
2.42.x
Other All
: Normal normal
: ---
Assigned To: libsoup-maint@gnome.bugs
libsoup-maint@gnome.bugs
: 709794 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2013-10-10 07:42 UTC by Ted
Modified: 2013-10-10 13:34 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Ted 2013-10-10 07:42:59 UTC
1. Assigning: "decoded" = storage returned from "g_strndup(part, length)"
2. leaked_storage: Variable "decoded" going out of scope leaks the storage it points to.

uri_decoded_copy (const char *part, int length, int *decoded_length)
{
	unsigned char *s, *d;
	char *decoded = g_strndup (part, length);

	g_return_val_if_fail (part != NULL, NULL); <== memory issue

	s = d = (unsigned char *)decoded;

        ....

        return decoded;
}

------------------------ patch option 1 --------------------------
--- a/libsoup/soup-uri.c
+++ b/libsoup/soup-uri.c
@@ -724,7 +724,7 @@ uri_decoded_copy (const char *part, int length, int *decoded_length)
     unsigned char *s, *d;
     char *decoded = g_strndup (part, length);

-    g_return_val_if_fail (part != NULL, NULL);
+    g_return_val_if_fail (decoded != NULL, NULL);

     s = d = (unsigned char *)decoded;
     do {

------------------------ patch option 2 --------------------------
--- a/libsoup/soup-uri.c
+++ b/libsoup/soup-uri.c
@@ -722,10 +722,13 @@ char *
 uri_decoded_copy (const char *part, int length, int *decoded_length)
 {
     unsigned char *s, *d;
-    char *decoded = g_strndup (part, length);

     g_return_val_if_fail (part != NULL, NULL);

+    char *decoded = g_strndup (part, length);
+
+    g_return_val_if_fail (decoded != NULL, NULL);
+
     s = d = (unsigned char *)decoded;
     do {
        if (*s == '%') {
Comment 1 Dan Winship 2013-10-10 13:15:27 UTC
*** Bug 709794 has been marked as a duplicate of this bug. ***
Comment 2 Dan Winship 2013-10-10 13:29:33 UTC
(In reply to comment #0)
> 1. Assigning: "decoded" = storage returned from "g_strndup(part, length)"
> 2. leaked_storage: Variable "decoded" going out of scope leaks the storage it
> points to.

I assume that's the output of some analysis tool?

g_return_if_fail()/g_return_val_if_fail() indicate a programmer error, so this wouldn't really be considered a leak, since only already-buggy programs will hit it.

As of glib 2.38, g_return_val_if_fail() is annotated so that clang's analyzer won't warn about things like this: The g_return_if_fail_warning() function that g_return_val_if_fail() calls is annotated with G_ANALYZER_NORETURN, which expands to __attribute__((analyzer_noreturn)) under clang (which makes clang pretend that g_return_if_fail_warning() never returns, for purposes of code analysis, but not for actual code generation).

If there's some similar way to provide hints to whatever analysis tool you're using, then we could possibly update the macros to do that as well.


Having said all that, it's trivial to just rewrite the code here to do the g_strndup() after the g_return_val_if_fail(), so I'll do that.
Comment 3 Dan Winship 2013-10-10 13:34:24 UTC
Fixed in git master (and in next week's 2.44.1 release)