GNOME Bugzilla – Bug 674592
Unreachable error handling code in gsocket.c
Last modified: 2012-04-25 14:51:40 UTC
Created attachment 212579 [details] Screenshot from Vigilant's Sentry tool. Another defect was recently picked up by Sentry (our static analysis tool) in glib's gsocket.c, around line 503 in g_socket_create_socket(): if (family < 0) { g_set_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT, _("Unable to create socket: %s"), _("Unknown family was specified")); return -1; } 'family' is an enum type (GSocketFamily) and the signedness of enum is implementation-dependent (unless the enum itself contains a negative value). On GCC, enum happens to be unsigned by default. So the conditional (family < 0) can never be true, and this error handling code is non-functional. Best, Mike
The enum type may have negative values on some platforms, just not on UNIX. Anyway, "0" is invalid too though. Does it complain about "family <= 0" ?
Sentry would not complain if the condition was <= 0. The way the enum is written: http://developer.gnome.org/gio/2.30/GSocketAddress.html#GSocketFamily leads me to believe that this is essentially the same as testing for == G_SOCKET_FAMILY_INVALID, which might be clearer. (Unless you want to handle users passing random negative integer values as well, but as I indicated above, these would be implicitly cast to a positive unsigned integer and the test would not catch them.)
> The way the enum is written leads me to believe that this is > essentially the same as testing for == G_SOCKET_FAMILY_INVALID no, as mentioned above, some of the GSocketFamily values may be negative *on some platforms*, which is what the test was originally there to catch. But it should have been catching G_SOCKET_FAMILY_INVALID too anyway, so using <= 0 catches both cases. Fixed in master. Thanks.