One more warning later on:

cc1: warnings being treated as errors
gstidentity.c: In function 'gst_identity_chain':
gstidentity.c:361: warning: value computed is not used

(a)
The first bunch should be fixed in CVS:

2006-01-04  Tim-Philipp Muller  <tim at centricular dot net>

        * gst/gstcaps.h:
        * gst/gstdata.h:
        * gst/gstprobe.h:
        * gst/gststructure.h:
        * gst/gsttaginterface.h:
          Don't use G_GNUC_CONST for _get_type() functions (see #324530
          and http://wingolog.org/archives/2005/03/24/98 for why); yes, we
          aim to please even unreleased and mind-boggingly cranky compilers.


Not sure yet how to tell gcc-4.1 that it's okay to ignore the return value of gst_foo_bar_ref()/unref().


(b)
jhbuild should probably disable -Werror when building GStreamer from CVS, as per our FAQ, see also http://bugzilla.gnome.org/show_bug.cgi?id=324530#c6 .

Regarding (b), sounds like a compiler bug to me.. unsure.

Note that gst_buffer_ref (buf) expands to

(GstBuffer*)(gst_data_ref ((GstData*)(buf)))

And gst_data_ref is not a const function or anything like that. Perhaps it's the cast of the return value that's bothering the compiler? Note this same problem should show up with 0.10 as well. Sounds like an overly cranky compiler to me.

Created attachment 57467 [details]
test case for compiler warning

OK attaching a brief test case that shows this issue. Compile with

gcc -Wall -c -o test test.c

The problem appears to be the cast in the return value. Not sure why the GCC folks consider this to mean that the return value is necessary.

Pushed upstream: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25810

Updating version, because it affects HEAD as well.

Gah, upstream marked as a dupe of http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24900, which links to http://gcc.gnu.org/ml/gcc-patches/2005-08/msg00275.html which claims this is intended behavior. Must think more about this.

I suppose we'll have to make gst_buffer_ref() and gst_event_ref() either static inline functions then, or proper functions, no?

Attached is a patch for making core compile with gcc 4.1 without any warnings.

It seems to work fine although I get an error when running make check:

Running suite(s): LibsABINo structure size list was generated for this architecture
ignoring

100%: Checks: 1, Failures: 0, Errors: 0
PASS: libs/libsabi
Running suite(s): GstBaseSrc
50%: Checks: 4, Failures: 0, Errors: 2
libs/basesrc.c:164:E:general:basesrc_eos_events_push: (after this point) Test timeout expired
libs/basesrc.c:94:E:general:basesrc_eos_events_push_live_op: (after this point) Test timeout expired
FAIL: libs/basesrc
Running suite(s): Controller
100%: Checks: 15, Failures: 0, Errors: 0
PASS: libs/controller
Running suite(s): data protocol
100%: Checks: 4, Failures: 0, Errors: 0
PASS: libs/gdp
Running suite(s): GstNetClientClock
100%: Checks: 2, Failures: 0, Errors: 0
PASS: libs/gstnetclientclock
Running suite(s): GstNetTimeProvider
100%: Checks: 2, Failures: 0, Errors: 0
PASS: libs/gstnettimeprovider
====================
1 of 39 tests failed
====================

Created attachment 61544 [details] [review]
Patch for core compilation with gcc 4.1

foo = gst_blah_ref(foo) is not appropriate for working around these warnings.

A (void) cast would be ok... except these are going to bite everyone USING gstreamer as well. We should do as recommended and use statement expressions to work around these (for gcc; for non-gcc the existing macros will be fine).

The last two hunks in the file are wrong, I assume you didn't mean to include them.

Created attachment 61553 [details] [review]
Updated version

This is a much saner patch.

The main modifications are:
_ making gst_[buffer|event|message]_ref() static inline functions.
_ Handling dereferenced pointers correctly.

Created attachment 61554 [details] [review]
patch for gst-plugins-base

Created attachment 61555 [details] [review]
patch for gst-plugins-good

I think it would be good to separate out your core patch into the GstObject** type-punning one and the one for gst_buffer_ref and friends.

I'm not sure that "static inline" is portable; would have to check. I'm sure that Mike will have more opinions aboot this...

'static inline' itself isn't portable, but __tim said that glib defines 'inline' to something appropriate; I didn't confirm this.

I suppose it'd be nice to see two seperate patches, but I'd like to see this go in sooner rather than later.

Created attachment 61601 [details] [review]
static inline functions for gst_[buffer|event|message]_ref()

Created attachment 61602 [details] [review]
other gcc4.1 fixes for core.

Second part of core patch split up

Created attachment 61686 [details] [review]
More efficient version of gst_[buffer|event|message]_ref()

The previous version was doing typechecking (using the GST_[BUFFER|EVENT|MESSAGE] macro). This is not necessary in fact, so a simple cast while do.

core is now fixed.

2006-03-21  Edward Hervey  <edward@fluendo.com>

	reviewed by: <delete if not using a buddy>

	* gst/gstbin.c: (gst_bin_dispose), (gst_bin_provide_clock_func),
	(gst_bin_handle_message_func):
	* gst/gstclock.c: (gst_clock_dispose), (gst_clock_set_master):
	* gst/gstelement.c: (gst_element_set_clock), (gst_element_dispose),
	(gst_element_set_bus_func):
	* gst/gstghostpad.c: (gst_proxy_pad_dispose):
	* gst/gstminiobject.c: (gst_value_set_mini_object),
	(gst_value_take_mini_object):
	* gst/gstpad.c: (gst_pad_set_pad_template):
	* gst/gstpipeline.c: (gst_pipeline_dispose),
	(gst_pipeline_use_clock), (gst_pipeline_auto_clock):
	* libs/gst/base/gstcollectpads.c: (gst_collect_pads_pop),
	(gst_collect_pads_chain):
	* libs/gst/net/gstnettimeprovider.c:
	(gst_net_time_provider_set_property):
	Series of fixes for dereferenced pointers that gcc 4.1 complains about.
	It's in fact all issues with gst_*object_replace().


2006-03-21  Edward Hervey  <edward@fluendo.com>

	* gst/gstbuffer.h:
	* gst/gstevent.h:
	* gst/gstmessage.h:
	gst_[buffer|event|message]_ref() macros are replaced by a static
	inline functions because gcc-4.1 will about if the return value
	isn't used.
	* tests/check/gst/gstevent.c: (event_probe):
	gst_event_ref now has to be given a GstEvent* , fix check accordingly.

And now -base is gcc 4.1 friendly

2006-03-21  Edward Hervey  <edward@fluendo.com>

	* ext/ogg/gstoggdemux.c: (gst_ogg_pad_dispose):
	* gst/playback/gstplaybin.c: (handoff):
	* gst/playback/gststreamselector.c:
	(gst_stream_selector_set_property):
	gcc 4.1 unreferenced pointer fixes.
	* sys/ximage/ximagesink.c: (gst_ximagesink_ximage_put):
	* sys/xvimage/xvimagesink.c: (gst_xvimagesink_xvimage_put):
	gst_buffer_ref() now takes a GstBuffer*.

-good is gcc 4.1 friendly.

2006-03-21  Edward Hervey  <edward@fluendo.com>

	* gst/apetag/gsttagdemux.c: (gst_tag_demux_reset):
	* gst/id3demux/gstid3demux.c: (gst_id3demux_reset):
	* gst/wavparse/gstwavparse.c: (gst_wavparse_create_sourcepad),
	(gst_wavparse_stream_headers), (gst_wavparse_send_event),
	(gst_wavparse_change_state):
	gcc 4.1 unreferenced pointer fixes.

Everything fixed, closing bug.

*** Bug 335828 has been marked as a duplicate of this bug. ***

*** Bug 325604 has been marked as a duplicate of this bug. ***