if (!mir_connection_is_valid(connection)) { mir_connection_release(connection); confuse_reader(); }

[Daniel van Vugt]

if (!mir_connection_is_valid(connection))
{
    mir_connection_release(connection);
    confuse_reader();
}

Apparently we've designed the client API such that even invalid connections need to be released. This is confusing because an "invalid" object is not something that normally needs releasing.

I'm told this is because the connection still represents an error object which needs releasing. This is a somewhat valid excuse, but we still have an ambiguity that needs solving:

If a connection is "invalid" due to being an uninitialized pointer then we need to stop the user from calling mir_connection_release (which will probably crash). If the connection is "invalid" due to proper initialization but a failed connection then indeed something still needs freeing.

Even if we solve this in libmirclient with some internal mir_connection_is_really_truly_valid(), then the issue of a confusing API design that's non-obvious and people are likely to mis-use remains... so something needs improving in the design and/or naming. Not just documentation.

Suggestion: If we do keep the current behaviour, then we could solve the confusion just by renaming:
   mir_connection_is_valid
to
   mir_connection_is_connected

[Michał Sawicz (saviq)]

Syncing task from Mir.