Bug 8164

Summary: enable -Wtype-limits or document why not
Product: dbus Reporter: frederic heem <frederic.heem>
Component: coreAssignee: D-Bus Maintainers <dbus>
Status: RESOLVED MOVED QA Contact: D-Bus Maintainers <dbus>
Severity: minor    
Priority: low CC: msniko14
Version: 1.5   
Hardware: All   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments: Fixes all the unused variable/functions, undeclared functions, and signedness warnings

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 
signedness
dbus-marshal-header.c: In function '_dbus_header_get_flag':
dbus-marshal-header.c:1450: warning: pointer targets in assignment differ in 
signedness
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.