[Bug 51356] Fix C warnings in the Telepathy tests library

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=51356

--- Comment #8 from Philip Withnall <bugzilla at tecnocode.co.uk> 2012-08-19 14:50:42 UTC ---
(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > It allows gcc to warn about missing cases in the switch statement
> > 
> > That's not fixing a warning, that's introducing a new warning and then fixing
> > it :-P
> 
> True. It stems from the first version of this patch, where I added a default
> case (as per -Wswitch-default) but didn’t add a case for CONNECTING, causing
> things to fail.
> 
> > Given that the default behaviour of an unhandled case in a switch is "do
> > nothing", and that's entirely appropriate for a change to a status you don't
> > recognise, I don't think the current code is wrong.
> > 
> > (Thought experiment: what would happen if Telepathy 1.0 added a DISCONNECTING
> > status, or a NEARLY_THERE status, or something? Answer: it would be reasonable
> > for nothing to happen when arriving at that status, the same as is currently
> > done for CONNECTING. So "do nothing by default" seems good.)
> 
> The advantage I see in -Wswitch-enum is its value when adding new values to
> enum types which _should_ be handled everywhere. For example, imagine an ERROR
> status, which should cause all clients to return immediately. A do-nothing
> default case isn’t going to work for that. The value of -Wswitch-enum is that
> it enables the compiler to warn you about every location where you’ve forgotten
> to handle the new ERROR value.
> 
> > > it does have a fairly
> > > low signal-to-noise ratio.
> > 
> > Yes, that's my objection. I think it's more likely that sprinkling mutable
> > casts through the code causes a bug to go undetected than that a string
> > constant is unintentionally modified.
> 
> I think it’s the other way round. Programmers must explicitly add casts to
> disable the warnings where there are false positives, which they should think
> about before adding. If the warning flag was disabled, all false negatives
> would go unnoticed.
> 
> > > If possible, a better fix would be to change the data structure
> > > to use const gchar* rather than gpointer for the affected field
> > 
> > This is an incompatible change (I think) but I would not necessarily object to
> > it for Telepathy 1.0. In this case I don't think the field is always a char
> > pointer (it's a user_data sort of thing) but gconstpointer would be OK.
> 
> I think gpointer → gconstpointer is _not_ an incompatible change. GLib has
> certainly done it for a few struct fields recently.

*Poke*

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA Contact for the bug.
You are the assignee for the bug.