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
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 *.
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.
Agree with Havoc, casting is just a very bad principle. It doesn't solve anything, worst, it hides some (nasty) bugs.
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).
(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.
-- 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.