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 667655 - check: elements/jifmux.c compile failure because of fail() calls with no arguments
check: elements/jifmux.c compile failure because of fail() calls with no argu...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Solaris
: Normal normal
: 0.10.37
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-01-10 19:50 UTC by Tim Mooney
Modified: 2012-01-20 18:25 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Tim Mooney 2012-01-10 19:50:42 UTC
I searched the bug database using several different terms and didn't turn up any previous report for this.

I'm building gst-plugins-bad-0.10.22 on x86_64-sun-solaris2.10 with the Oracle Studio 12.2 compilers.

I get a compile failure in elements/jifmux.c when running gmake check:

        source='elements/jifmux.c' object='elements_jifmux-jifmux.o' libtool=no \
        DEPDIR=.deps depmode=none /bin/bash ../../depcomp \
        cc -xipo=2 -xO4 -xc99=all -DHAVE_CONFIG_H -I. -I../..   -I/local/gnu/include -I/local/gnu/include -I/local/include -D_REENTRANT -D_PTHREADS -I/local/gnu/include/gstreamer-0.10 -I/local/gnu/include/glib-2.0 -I/local/gnu/lib/64/glib-2.0/include -I/local/gnu/include/libxml2   -I/local/include   -D_REENTRANT -D_PTHREADS -I/local/gnu/include/gstreamer-0.10 -I/local/gnu/include/glib-2.0 -I/local/gnu/lib/64/glib-2.0/include -I/local/gnu/include/libxml2      -g    -DGST_TEST_FILES_PATH="\"../../tests/files\"" -UG_DISABLE_ASSERT -UG_DISABLE_CAST_CHECKS -Xa -xO4 -xipo=2 -features=extensions -KPIC -m64 -xtarget=native -xarch=native -I/local/gnu/include -I/local/gnu/include -I/local/include -c -o elements_jifmux-jifmux.o `test -f 'elements/jifmux.c' || echo './'`elements/jifmux.c
"/local/gnu/include/glib-2.0/gobject/gparam.h", line 157: warning: integer overflow detected: op "<<"
"elements/jifmux.c", line 343: syntax error before or at: ,
"elements/jifmux.c", line 343: syntax error before or at: ,
"elements/jifmux.c", line 343: syntax error before or at: ,
"elements/jifmux.c", line 345: syntax error before or at: ,
"elements/jifmux.c", line 345: syntax error before or at: ,
"elements/jifmux.c", line 345: syntax error before or at: ,
"elements/jifmux.c", line 347: syntax error before or at: ,
"elements/jifmux.c", line 347: syntax error before or at: ,
"elements/jifmux.c", line 347: syntax error before or at: ,
"elements/jifmux.c", line 349: syntax error before or at: ,
"elements/jifmux.c", line 349: syntax error before or at: ,
"elements/jifmux.c", line 349: syntax error before or at: ,
"elements/jifmux.c", line 351: syntax error before or at: ,
"elements/jifmux.c", line 351: syntax error before or at: ,
"elements/jifmux.c", line 351: syntax error before or at: ,

and so on.  It's for every call of the GST_COMPARE_GST_STRING_TAG_TO_EXIF_SHORT_FUNC utility macro.

Preprocessing the file and examining the results, the issue is with each call to "fail()" in the resulting function.

The problem is that fail() is not being passed any arguments at all, so the resulting _fail_unless call has nothing for the ## __VA_ARGS__ position (from the installed gst/check/internal-check.h header, which comes from gstreamer 0.10.35).

If I fill in some value in the calls to fail() in that macro, and the two other areas in jifmux.c where fail() is called with no arguments, the compile proceeds.
Comment 1 Vincent Penquerc'h 2012-01-11 14:34:01 UTC
You appear not to be using GCC, from looking at the arguments to cc, and fail uses a GCC extension (##), which will remove the extraneous comma if the varargs are empty.
The jifmux test is the only piece of code I can see which uses the fail call, and every time without any arguments, so maybe fail could be changed to not allow any arguments, eg:

#define fail() _fail_unless(0, __FILE__, __LINE__, "Failed" , NULL)

That might break external code, if any actually uses varargs here.
Comment 2 Tim Mooney 2012-01-11 23:07:08 UTC
You're right, as I mentioned at the top of the ticket, I'm using the no-cost Sun/Oracle Studio compiler toolchain.

I didn't realize that ## is a gcc-only extension.  It would be really nice to fix this at the source; make fail() work for both gcc and non-gcc, even if it means something like

#if defined(__GNUC__)
# define fail current_implementation
#else
# define fail some_alternate_implementation
#endif

I've been doing some reading on check, and it looks like the fail/fail_if/fail_unless interface may be deprecated in favor of the ck_abort/ck_abort_msg/ck_assert/ck_assert_msg API.

I'll keep looking at check() to see if I can come up with something better that might be accepted upstream too.

In the meantime, I just hacked jifmux.c so that fail() is always called as fail("").
Comment 3 Vincent Penquerc'h 2012-01-18 15:17:32 UTC
In fact, I think it might be as simple as this patch, since the first argument, if present, would be a format string. Does this make it work for you ? If so we won't need different definitions.


diff --git a/libs/gst/check/libcheck/check.h.in b/libs/gst/check/libcheck/check.h.in
index 231fdbb..3ed1acd 100644
--- a/libs/gst/check/libcheck/check.h.in
+++ b/libs/gst/check/libcheck/check.h.in
@@ -232,7 +232,7 @@ static void __testname (int _i CK_ATTRIBUTE_UNUSED)\
         "Failure '"#expr"' occured" , ## __VA_ARGS__, NULL)
 
 /* Always fail */
-#define fail(...) _fail_unless(0, __FILE__, __LINE__, "Failed" , ## __VA_ARGS__, NULL)
+#define fail(...) _fail_unless(0, __FILE__, __LINE__, "Failed"  __VA_ARGS__, NULL)
 
 /* Non macro version of #fail_unless, with more complicated interface */
 void CK_EXPORT _fail_unless (int result, const char *file,
Comment 4 Tim Mooney 2012-01-20 01:53:50 UTC
Your patch works Vincent.  The test results in a failure:

Running suite(s): jifmux
0%: Checks: 1, Failures: 1, Errors: 0
elements/jifmux.c:925:F:general:test_jifmux_tags:0: Assertion 'test_data.result'
 failed
FAIL: elements/jifmux


but at least now it can be compiled.
Comment 5 Vincent Penquerc'h 2012-01-20 18:11:39 UTC
Pushed, will be upstreamed when I find anything else than libraries for checking libraries or the GStreamer copy itself :)


commit 28d5e8150369efb477a25a4e701e62281fbecebc
Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk>
Date:   Fri Jan 20 18:06:04 2012 +0000

    libcheck: make the definition of fail not fail with non GCC compilers
Comment 6 Tim-Philipp Müller 2012-01-20 18:24:57 UTC
Hrm, not sure about this.

a) vararg macros aren't portable in the first place, technically we are supposed to support anything C89, so would need to provide multiple versions depending on what the compiler supports, or use a vararg function as backup. (see e.g. gstinfo.h)

b) I don't know if we really need to allow an argument-less fail() in the GStreamer world in the first place.

But I'm also happy to just leave it be until someone else complains.