GNOME Bugzilla – Bug 687385
Add some stricter CFLAGS, fix up the code
Last modified: 2012-11-03 12:20:40 UTC
I'd like to do this for gnome-common...but thought I'd have GLib do it first.
Created attachment 227847 [details] [review] build: Don't use C99 declarations Since GLib needs to compile with MSVC, we can't use them. This fixes compilation when using -Werror=declaration-after-statement.
Created attachment 227848 [details] [review] build: Ensure we #include header files for glib-genmarshal code Otherwise we fail with -Werror=missing-prototypes.
Created attachment 227849 [details] [review] build: Add missing "static" keyword where it should be used Otherwise we fail to build with -Werror=missing-prototypes.
Created attachment 227850 [details] [review] build: Prototype GType accessors for private classes Otherwise we fail to build with -Werror=missing-prototypes.
Created attachment 227851 [details] [review] Use (void) for no parameters, not () This ensures we build with -Werror=missing-parameter-type.
Created attachment 227852 [details] [review] tests/1bitmutex: Hack to build with -Werror=missing-prototypes Admittedly, this could probably be better, but it builds.
Created attachment 227853 [details] [review] g_check_setuid: Use G_GNUC_INTERNAL, include private header Otherwise we fail to build with -Werror=missing-prototypes.
Created attachment 227854 [details] [review] gslice: Prototype G_ENABLE_DEBUG function that's part of ABI Sadly, g_slice_debug_tree_statistics is conditionally part of the public ABI. We might as well make it conditionally part of the API as well, even though this will require people actually using it to
Created attachment 227855 [details] [review] gcharset: Add header file for private API This fixes the build with -Werror=missing-prototypes.
Created attachment 227856 [details] [review] gettext: Add missing include Fixes the build with -Werror=missing-prototypes.
Created attachment 227857 [details] [review] test-pipe-unix: Add missing include Fixes the build with -Werror=missing-prototypes.
Created attachment 227858 [details] [review] gdbusactiongroup: Add prototype for g_dbus_action_group_sync() Even private functions that are actually called across compilation units should have prototypes. For g_dbus_action_group_sync(), create one in gdbusactiongroup-private.h
Created attachment 227859 [details] [review] gdateparser: Delete unused debug print function
Created attachment 227860 [details] [review] configure: Enable set of standard -Werror=foo flags We're not going to depend on gnome-common (I assume) so this patch nicks the systemd macro to test for compiler flags, and uses it to set a similar set of -Werror=foo as the gnome-common one does. See https://bugzilla.gnome.org/show_bug.cgi?id=608953 See https://mail.gnome.org/archives/desktop-devel-list/2012-July/msg00100.html If we're going to be setting more strict compiler flags for GNOME, we should really ensure GLib builds with them first, as it's kind of the model citizen. In particular, you can see several times that downstreams such as Debian have come in and fixed -Wformat-security bugs. We should never let those get into tarballs, or even commits.
Review of attachment 227847 [details] [review]: sure
Review of attachment 227848 [details] [review]: fine
Review of attachment 227849 [details] [review]: looks fine
Review of attachment 227850 [details] [review]: hmm, not sure I love this one, but ok
Review of attachment 227851 [details] [review]: sure
Review of attachment 227852 [details] [review]: yay, pragma :-/
Review of attachment 227853 [details] [review]: why didn't this fail abicheck ?
Review of attachment 227854 [details] [review]: ok
Review of attachment 227855 [details] [review]: fine
Review of attachment 227856 [details] [review]: ok
Review of attachment 227857 [details] [review]: sure
Review of attachment 227858 [details] [review]: yes
Review of attachment 227859 [details] [review]: yay less code
Review of attachment 227860 [details] [review]: ok
(In reply to comment #21) > Review of attachment 227853 [details] [review]: > > why didn't this fail abicheck ? Ah, it is marked that way in the header - we just needed to #include "glib-private.h". Will adjust patch and commit message accordingly.
Attachment 227847 [details] pushed as 67466b4 - build: Don't use C99 declarations Attachment 227848 [details] pushed as f6da43f - build: Ensure we #include header files for glib-genmarshal code Attachment 227849 [details] pushed as 6d88a2f - build: Add missing "static" keyword where it should be used Attachment 227850 [details] pushed as 84475e4 - build: Prototype GType accessors for private classes Attachment 227851 [details] pushed as 8e59d86 - Use (void) for no parameters, not () Attachment 227852 [details] pushed as 1398927 - tests/1bitmutex: Hack to build with -Werror=missing-prototypes Attachment 227854 [details] pushed as 488cdb1 - gslice: Prototype G_ENABLE_DEBUG function that's part of ABI Attachment 227855 [details] pushed as 4c2a659 - gcharset: Add header file for private API Attachment 227856 [details] pushed as 3686aa0 - gettext: Add missing include Attachment 227857 [details] pushed as dc4922a - test-pipe-unix: Add missing include Attachment 227858 [details] pushed as 94ef8df - gdbusactiongroup: Add prototype for g_dbus_action_group_sync() Attachment 227859 [details] pushed as 055aa2b - gdateparser: Delete unused debug print function Attachment 227860 [details] pushed as 28b30ca - configure: Enable set of standard -Werror=foo flags
Review of attachment 227850 [details] [review]: I do this static pre-declaration of _get_type() functions in testcases all the time specifically to avoid the warning in this case. I considered that maybe we want a G_DEFINE_PRIVATE_TYPE() for this reason...
Review of attachment 227852 [details] [review]: No nicer way to do this? What's the problem?