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 590970 - [souphttpsrc] better fix for compiler warning fix
[souphttpsrc] better fix for compiler warning fix
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal blocker
: 0.10.16
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2009-08-06 16:11 UTC by Tim-Philipp Müller
Modified: 2009-08-06 20:44 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Tim-Philipp Müller 2009-08-06 16:11:11 UTC
This was committed presumably to work around bug #588771 (strict aliasing compiler warnings):

 commit 4e6fcd2345d208d029e46286c141a2f6b4ea5d7d
 Author: Edward Hervey <bilboed@bilboed.com>
 Date:   Mon Jul 27 18:44:45 2009 +0200

    soup: Use "GET" instead of SOUP_METHOD_GET. Fixes build (...)
    (...)

diff --git a/ext/soup/gstsouphttpsrc.c b/ext/soup/gstsouphttpsrc.c
index c50fd3e..18b39b1 100644
--- a/ext/soup/gstsouphttpsrc.c
+++ b/ext/soup/gstsouphttpsrc.c
@@ -1101,7 +1101,7 @@ gst_soup_http_src_parse_status (SoupMessage * msg, GstSoupHTTPSrc * src)
 static gboolean
 gst_soup_http_src_build_message (GstSoupHTTPSrc * src)
 {
-  src->msg = soup_message_new (SOUP_METHOD_GET, src->location);
+  src->msg = soup_message_new ("GET", src->location);
   if (!src->msg) {
     GST_ELEMENT_ERROR (src, RESOURCE, OPEN_READ,
         (NULL), ("Error parsing URL \"%s\"", src->location));


I'm not convinced this change is technically entirely correct, and think we should try to fix this differently somehow so it doesn't come back to bite us in future.

In particular, the way the API works:

 - libsoup might now or in future rely on the
   first argument to be an interned string

 - libsoup might now or in future rely on the
   exported symbol _SOUP_METHOD_GET having been
   initialised to a valid, possibly interned,
   string after soup_message_new() has been called

 - libsoup might now or in future rely on being able
   to do direct pointer comparisons between the
   method string passed and e.g. _SOUP_METHOD_GET

I don't know if any of this is actually used, but that's the way the API works, and we shouldn't just ignore that (and given the commit message I'm fairly sure this wasn't checked). Until libsoup git is changed to something like #define SOUP_METHOD_GET "GET" I'm tempted to think there's a reason for all this inlining and variable initialising etc.

Since I haven't seen the warning myself with the version of libsoup that I have, it'd be nice if someone who else who did have the warning could look for a better fix.
Comment 1 Sebastian Dröge (slomo) 2009-08-06 19:11:06 UTC
The reason for this is probably that only a pointer comparison is needed (and even a switch statement could be used)... This should be reverted before the release IMHO. To get around the compiler warning we could add -fno-strict-aliasing inside the soup plugin's Makefile.am to CFLAGS. I don't think this should be added globally again...
Comment 2 Sebastian Dröge (slomo) 2009-08-06 19:25:00 UTC
This is fixed in libsoup GIT: http://git.gnome.org/cgit/libsoup/commit
/?id=05b6c532d2f0eade1774a6c6fd6693b51cf90b8d

commit c42f0ad5b6aefd42a8336bd73d6e856fce306908
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Thu Aug 6 21:24:14 2009 +0200

    souphttpsrc: Use SOUP_METHOD_GET instead of "GET" string
    
    Fixes bug #590970.
Comment 3 Tim-Philipp Müller 2009-08-06 20:44:12 UTC
There's going to be a new libsoup release with the fix on Monday (Aug 10), so we think it's not really worth hacking around this problem, esp. seeing that the version used is a development version and it only happens for this particular version which will be replaced in a few days.

If someone else feels like adding -fno-strict-aliasing to the CFLAGS or hacking around it some other way just for this particular version, please feel free.