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 687385 - Add some stricter CFLAGS, fix up the code
Add some stricter CFLAGS, fix up the code
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2012-11-02 00:12 UTC by Colin Walters
Modified: 2012-11-03 12:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
build: Don't use C99 declarations (4.34 KB, patch)
2012-11-02 00:12 UTC, Colin Walters
committed Details | Review
build: Ensure we #include header files for glib-genmarshal code (2.00 KB, patch)
2012-11-02 00:12 UTC, Colin Walters
committed Details | Review
build: Add missing "static" keyword where it should be used (4.81 KB, patch)
2012-11-02 00:12 UTC, Colin Walters
committed Details | Review
build: Prototype GType accessors for private classes (6.01 KB, patch)
2012-11-02 00:12 UTC, Colin Walters
committed Details | Review
Use (void) for no parameters, not () (1.55 KB, patch)
2012-11-02 00:12 UTC, Colin Walters
committed Details | Review
tests/1bitmutex: Hack to build with -Werror=missing-prototypes (1.01 KB, patch)
2012-11-02 00:12 UTC, Colin Walters
committed Details | Review
g_check_setuid: Use G_GNUC_INTERNAL, include private header (993 bytes, patch)
2012-11-02 00:12 UTC, Colin Walters
committed Details | Review
gslice: Prototype G_ENABLE_DEBUG function that's part of ABI (979 bytes, patch)
2012-11-02 00:13 UTC, Colin Walters
committed Details | Review
gcharset: Add header file for private API (2.57 KB, patch)
2012-11-02 00:13 UTC, Colin Walters
committed Details | Review
gettext: Add missing include (622 bytes, patch)
2012-11-02 00:13 UTC, Colin Walters
committed Details | Review
test-pipe-unix: Add missing include (709 bytes, patch)
2012-11-02 00:13 UTC, Colin Walters
committed Details | Review
gdbusactiongroup: Add prototype for g_dbus_action_group_sync() (4.03 KB, patch)
2012-11-02 00:13 UTC, Colin Walters
committed Details | Review
gdateparser: Delete unused debug print function (997 bytes, patch)
2012-11-02 00:13 UTC, Colin Walters
committed Details | Review
configure: Enable set of standard -Werror=foo flags (12.36 KB, patch)
2012-11-02 00:13 UTC, Colin Walters
committed Details | Review

Description Colin Walters 2012-11-02 00:12:36 UTC
I'd like to do this for gnome-common...but thought I'd have GLib do it
first.
Comment 1 Colin Walters 2012-11-02 00:12:40 UTC
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.
Comment 2 Colin Walters 2012-11-02 00:12:43 UTC
Created attachment 227848 [details] [review]
build: Ensure we #include header files for glib-genmarshal code

Otherwise we fail with -Werror=missing-prototypes.
Comment 3 Colin Walters 2012-11-02 00:12:45 UTC
Created attachment 227849 [details] [review]
build: Add missing "static" keyword where it should be used

Otherwise we fail to build with -Werror=missing-prototypes.
Comment 4 Colin Walters 2012-11-02 00:12:48 UTC
Created attachment 227850 [details] [review]
build: Prototype GType accessors for private classes

Otherwise we fail to build with -Werror=missing-prototypes.
Comment 5 Colin Walters 2012-11-02 00:12:51 UTC
Created attachment 227851 [details] [review]
Use (void) for no parameters, not ()

This ensures we build with -Werror=missing-parameter-type.
Comment 6 Colin Walters 2012-11-02 00:12:53 UTC
Created attachment 227852 [details] [review]
tests/1bitmutex: Hack to build with -Werror=missing-prototypes

Admittedly, this could probably be better, but it builds.
Comment 7 Colin Walters 2012-11-02 00:12:58 UTC
Created attachment 227853 [details] [review]
g_check_setuid: Use G_GNUC_INTERNAL, include private header

Otherwise we fail to build with -Werror=missing-prototypes.
Comment 8 Colin Walters 2012-11-02 00:13:00 UTC
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
Comment 9 Colin Walters 2012-11-02 00:13:03 UTC
Created attachment 227855 [details] [review]
gcharset: Add header file for private API

This fixes the build with -Werror=missing-prototypes.
Comment 10 Colin Walters 2012-11-02 00:13:05 UTC
Created attachment 227856 [details] [review]
gettext: Add missing include

Fixes the build with -Werror=missing-prototypes.
Comment 11 Colin Walters 2012-11-02 00:13:08 UTC
Created attachment 227857 [details] [review]
test-pipe-unix: Add missing include

Fixes the build with -Werror=missing-prototypes.
Comment 12 Colin Walters 2012-11-02 00:13:11 UTC
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
Comment 13 Colin Walters 2012-11-02 00:13:14 UTC
Created attachment 227859 [details] [review]
gdateparser: Delete unused debug print function
Comment 14 Colin Walters 2012-11-02 00:13:17 UTC
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.
Comment 15 Matthias Clasen 2012-11-02 11:56:39 UTC
Review of attachment 227847 [details] [review]:

sure
Comment 16 Matthias Clasen 2012-11-02 11:57:02 UTC
Review of attachment 227848 [details] [review]:

fine
Comment 17 Matthias Clasen 2012-11-02 12:03:30 UTC
Review of attachment 227849 [details] [review]:

looks fine
Comment 18 Matthias Clasen 2012-11-02 12:04:44 UTC
Review of attachment 227850 [details] [review]:

hmm, not sure I love this one, but ok
Comment 19 Matthias Clasen 2012-11-02 12:05:20 UTC
Review of attachment 227851 [details] [review]:

sure
Comment 20 Matthias Clasen 2012-11-02 12:05:55 UTC
Review of attachment 227852 [details] [review]:

yay, pragma :-/
Comment 21 Matthias Clasen 2012-11-02 12:06:35 UTC
Review of attachment 227853 [details] [review]:

why didn't this fail abicheck ?
Comment 22 Matthias Clasen 2012-11-02 12:07:10 UTC
Review of attachment 227854 [details] [review]:

ok
Comment 23 Matthias Clasen 2012-11-02 12:07:36 UTC
Review of attachment 227855 [details] [review]:

fine
Comment 24 Matthias Clasen 2012-11-02 12:08:03 UTC
Review of attachment 227856 [details] [review]:

ok
Comment 25 Matthias Clasen 2012-11-02 12:08:21 UTC
Review of attachment 227857 [details] [review]:

sure
Comment 26 Matthias Clasen 2012-11-02 12:08:50 UTC
Review of attachment 227858 [details] [review]:

yes
Comment 27 Matthias Clasen 2012-11-02 12:09:15 UTC
Review of attachment 227859 [details] [review]:

yay less code
Comment 28 Matthias Clasen 2012-11-02 12:10:29 UTC
Review of attachment 227860 [details] [review]:

ok
Comment 29 Colin Walters 2012-11-02 12:57:25 UTC
(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.
Comment 30 Colin Walters 2012-11-02 13:05:16 UTC
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
Comment 31 Allison Karlitskaya (desrt) 2012-11-03 12:19:58 UTC
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...
Comment 32 Allison Karlitskaya (desrt) 2012-11-03 12:20:40 UTC
Review of attachment 227852 [details] [review]:

No nicer way to do this?  What's the problem?