Bug 8164 - enable -Wtype-limits or document why not
Summary: enable -Wtype-limits or document why not
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: 1.5
Hardware: All All
: low minor
Assignee: D-Bus Maintainers
QA Contact: D-Bus Maintainers
Depends on:
Reported: 2006-09-07 00:40 UTC by frederic heem
Modified: 2018-10-12 21:03 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:

Fixes all the unused variable/functions, undeclared functions, and signedness warnings (22.42 KB, patch)
2006-09-07 15:49 UTC, John (J5) Palmieri
Details | Splinter Review

Description frederic heem 2006-09-07 00:40:21 UTC
dbus doesn't compile with -Wall -Werror with gcc version 4.1.1 20060525 (Red 
Hat 4.1.1-1)
Here are some examples:
dbus-marshal-header.c: In function '_dbus_header_toggle_flag':
dbus-marshal-header.c:1429: warning: pointer targets in assignment differ in 
dbus-marshal-header.c: In function '_dbus_header_get_flag':
dbus-marshal-header.c:1450: warning: pointer targets in assignment differ in 
Comment 1 John (J5) Palmieri 2006-09-07 15:49:49 UTC
Created attachment 6875 [details] [review]
Fixes all the unused variable/functions, undeclared functions, and signedness warnings

I fixed the signedness errors by doing casts.  For most of the cases it was a
discrepency between an API which took unsigned char * but we were passing char
Comment 2 Havoc Pennington 2006-09-07 15:59:39 UTC
I hadn't fixed these because I don't think casts are really right. Probably 
those APIs should not take unsigned char. Alternatively, maybe some more of 
the marshaling code should be using unsigned char throughout. The general idea 
with the unsigned char was that it meant "binary string" but then dbus_string 
is used for both binary and nonbinary strings so it's just kind of a bad idea 
to use unsigned char at all probably.

Anyway, I'm not sure it's urgent to fix this, but I probably would not fix it 
by adding casts. The casts are really just not right and could hide other bugs.
Comment 3 frederic heem 2006-09-08 00:46:16 UTC
Agree with Havoc, casting is just a very bad principle. It doesn't solve 
anything, worst, it hides some (nasty) bugs.

Comment 4 Simon McVittie 2011-05-10 01:50:45 UTC
We compile with -Wall -Werror by suppressing some specific warnings, at the cost of possibly suppressing more warnings than we need to; so the remaining bug here is that we could get more help from the compiler by suppressing fewer warnings.

We use -Wno-pointer-sign and -Wno-sign-compare to silence the signedness warnings: after fixing Bug #17433 and Bug #17289 we may be able to remove those.

We use -Wno-unused to silence the unused-stuff warnings, although I'd prefer to use a smaller hammer, preferably just -Wno-unused-parameters.

Ideally I'd like to use the same warning flags as Telepathy, which includes -Wextra and an assortment of others (but then we explicitly silence -Wmissing-field-initializers, because we consider insufficient initializers causing the rest to be initialized to 0 to be a C feature not a bug, and -Wunused-parameters, because in async callbacks you often don't care about all parameters).
Comment 5 Simon McVittie 2013-10-08 12:14:30 UTC
(In reply to comment #4)
> Ideally I'd like to use the same warning flags as Telepathy

We more or less do that now. Here's what's left:

-Wno-pointer-sign is Bug #15522.

-Wno-sign-compare is Bug #17289.

-Wno-type-limits doesn't have its own bug. Let's use this one.
Comment 6 GitLab Migration User 2018-10-12 21:03:50 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/dbus/dbus/issues/2.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.