Bug 20496

> This is not true unless the
> fatal_warnings_on_check_failed flag was set to true at D-BUS compile time.

... which it is by default (Debian and derived distributions patch that out, but e.g. Fedora doesn't).

I think the correct wording to describe the implemented behaviour would be "it is an error to pass a type not mentioned in dbus-protocol.h to this function", leaving it an implementation detail whether that results in a crash or not. It's always considered to be an error if a check in libdbus fails, whether libdbus is configured to try to carry on or to abort; it's basically the same as g_return_if_fail(), g_assert_not_reached() and g_critical() in GLib.

Even better would be if we had an API function, dbus_bool_t dbus_is_type (int), which would return TRUE for (e.g.) 'i' or 'a', but not for 'f' (at the time of writing, anyway); then these functions could be documented as "it is an error if dbus_is_type() would return FALSE for the same parameter".

(In reply to comment #0)
> The documentation should be updated to reflect that use of these functions is
> safe for packaged copies of libdbus for use with unknown values.

That's not the intention, so, no.

(In reply to comment #1)
> I think the correct wording to describe the implemented behaviour would be "it
> is an error to pass a type not mentioned in dbus-protocol.h to this function",
> leaving it an implementation detail whether that results in a crash or not.

Fixed on my doc-typechecks-20496 branch, I'll attach the patch in a moment. I think this is good to merge for 1.4.

> Even better would be if we had an API function, dbus_bool_t dbus_is_type (int),
> which would return TRUE for (e.g.) 'i' or 'a', but not for 'f'

Fixed on my dbus_type_is_valid-20496 branch; this adds API, so I think we should only merged it into master (for 1.5).

Created attachment 44085 [details] [review]
dbus_type_is_basic etc.: it is an error to pass in bad typecodes

Previously, the comments said "this function will crash", but that's not
strictly true (checks can be disabled or made non-fatal). Their behaviour
is undefined if you do that, though.

(Review for 1.4 requested.)

Created attachment 44086 [details] [review]
Make dbus_type_is_valid into public API

This is just as useful for bindings as dbus_signature_validate, and I
think it's a good design principle to say that anything checked in a 
_dbus_return_if_fail should be something the caller could check 
for themselves. 

(Review for master requested.)

An alternative resolution would be to make these functions silently return FALSE (without a check) on invalid typecodes. This is arguably also new API, so should probably only be done in 1.5.

Review of attachment 44085 [details] [review]:

Looks fine.

Review of attachment 44086 [details] [review]:

Looks reasonable.

Thanks, fixed in git for 1.4.8, 1.5.0 (documentation) and 1.5.0 only (new API).