Code review comment for lp:~mir-team/mir/probe-client-drivers

On Thu, Dec 18, 2014 at 11:46 PM, Alan Griffiths <email address hidden>
wrote:
> 40 +#include <iostream>
> 41 #include <assert.h>
> 42
> 43 namespace mt = mir::test;
> 44 @@ -45,7 +46,11 @@
> 45 mir_display_output_id_invalid};
> 46
> 47 auto surface = mir_connection_create_surface_sync(connection,
> &surface_params);
> 48 - assert(mir_surface_is_valid(surface));
> 49 + if (!mir_surface_is_valid(surface))
> 50 + {
> 51 + std::cerr << "Surface creation failed: " <<
> mir_surface_get_error_message(surface) << std::endl;
> 52 + exit(1);
> 53 + }
> 54
> 55 return surface;
> 56 }
> 57 @@ -93,7 +98,11 @@
> 58 void TouchMeasuringClient::run(std::string const& connect_string)
> 59 {
> 60 auto connection = mir_connect_sync(connect_string.c_str(),
> "frame-uniformity-test");
> 61 - assert(mir_connection_is_valid(connection));
> 62 + if (!mir_connection_is_valid(connection))
> 63 + {
> 64 + std::cerr << "Connection to Mir failed: " <<
> mir_connection_get_error_message(connection) << std::endl;
> 65 + exit(1);
> 66 + }
>
> wouldn't "#undef NDEBUG" before including <cassert> be simpler and
> clearer?

No, because that wouldn't print any diagnostics as to why the
connection/surface was invalid. I added those when debugging why the
frame_uniformity test was assert()ing :)

There might be an argument for having mir_assert_*_is_valid convenience
functions in the client library that do this, now that we've got client
library logging!