Skip to content

Fix remaining compiler warnings

Related to #15 (closed)

This MR fixes or supresses all remaining warnings. Following this, I'll make a separate MR to enable -Werror that will resolve #15 (closed) (Just separating since it's a bit of a separate consideration that might have its own discussion)

There's a few distinct issues that were fixed in standalone commits, similar to !65 (merged). It'll be best to review commit by commit.

I don't think it's worth splitting into multiple MRs since I think it's a good size for review, and it's all working toward the single broad goal of Dealing with all remaining compiler warnings. There's just one major distinction in that 3rd party warnings are supressed, and our own are fixed.

A lot of issues are being patched over in this MR, but will be fixed in full in the gstreamer upgrade.
I've made issue #78 (closed) to follow up on warnings that Should be fixed by the gstreamer upgrade.

Enable IDs for warnings

I've added a flag that causes warnings to be reported with the relevant code/flag. I don't think there's any harm in that, and it makes suppressing 3rd party warnings simpler in future. Note there's one flag for gcc and one for nvcc.

3rd party warnings

Only 3 distinct 3rd party warnings needed suppressing, which were repeated thousands of times.

  • glib.h compiles with a warning in nvcc: warning: unrecognized format function type "gnu_printf" ignored
  • Gstreamer uses a deprecated mutex: warning: 'GStaticRecMutex' is deprecated: Use 'GRecMutex' instead
    • It will also likely be fixed by the gstreamer upgrade
  • _spiir_decomp.c seems to use deprecated numpy API. One of its includes adds this compiler warning: warning: #warning "Using deprecated NumPy API, disable it by " "#defining NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION".
    • That #define seems to add warnings and exclude some deprecated features. It's already in _postcohtable.c, but when enabling we hit a lot of additional warnings and errors that I don't think are worth attempting until the python upgrade.
    • I've suppressed the warning in the included header.
    • This probably won't be fixed just by upgrading dependencies, but will have been done as part of the python upgrade.

Our warnings

  • xmlStrdup can return NULL. Something about the way we were using its return value made the compiler give a 'maybe uninitialized' warning. I've added a null check there, printing an error message and breaking or continuing the read loop.
    • I wasn't sure how robust to make the error, or if it's a critical error or not.
    • My change has added yet more nesting. I've made #79 to figure out & refactor our ligolw classes, but don't want to tackle that without a dedicated issue.
  • string.h needs to be included after Python.h, since Python.h redefines a POSIX #define. You can read more here: https://bugs.python.org/issue1045893),
    • I don't like #includes having mandatory order, especially with autoformatters expecting to be able to sort includes.
    • BUT it's possible it goes away after the python upgrade.
  • Added an explicit cast from gint to guint to compare with another unsigned int.
    • The variable itself Should be guint already. This gets fixed in the gstreamer upgrade.
  • Moved -fpermissive to CXXFLAGS since it's invalid for C compilation.

Testing

I've tested here: /fred/oz016/tdavies/projects/testing/run_tdavies__fix_system_compiler_warnings There's one run without the review change, and another after.

Edited by Timothy Davies

Merge request reports