Merge lp:~mir-team/mir/size-t-display-config-api into lp:mir

[Brandon Schaefer]

Clean up display config returns. size_t is much more appropriate for an index and size of mm.

Clean up display config returns. size_t is much more appropriate for an index and size of mm.

[Daniel van Vugt (vanvugt)]

size_t's primary and traditional purpose is to represent a number of bytes. For other purposes that are just an integer number, int or unsigned types are more appropriate.

On that note, mir_output_get_{current,preferred}_mode_index need to be changed to return int, since they are allowed and expected to sometimes return -1.

review: Disapprove

[Daniel van Vugt (vanvugt)]

See the C language spec. size_t's purpose is to represent the result of 'sizeof' or 'offsetof'. It may also be used to represent multiples of bytes such as a number of structures or elements returned in memory. However the important point is that sizeof always refers to the memory size (or multiple of memory size) of something.

7.17 Common definitions <stddef.h>
1 The following types and macros are defined in the standard header <stddef.h>. Some
are also defined in other headers, as noted in their respective subclauses.
2 The types are
ptrdiff_t
which is the signed integer type of the result of subtracting two pointers;
size_t
which is the unsigned integer type of the result of the sizeof operator;

http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1124.pdf

[Daniel van Vugt (vanvugt)]

Hmm, by that definition a number or index of structures may use size_t. Just not a number of millimetres.

(1) So in the least size_t needs to be removed from:

    mir_output_get_physical_height_mm
    mir_output_get_physical_width_mm

(2) Also this is not portable since some platforms won't have sizeof(unsigned long) == sizeof(size_t):

219 - int output_id;
220 - if (++action < action_end && 1 == sscanf(*action, "%d", &output_id))
221 + size_t output_id;
222 + if (++action < action_end && 1 == sscanf(*action, "%lu", &output_id))

(3) To avoid forcing downstreams to cast, it would be helpful to make this int or ssize_t:

   current_mode_index
   preferred_mode_index

because they are allowed to be and often are negative.

review: Needs Fixing

[Mir CI Bot (mir-ci-bot)]

FAILED: Continuous integration, rev:3964
https://mir-jenkins.ubuntu.com/job/mir-ci/2790/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/3654/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3733
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3723
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3723
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/3723
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/3681/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3681
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3681/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/3681
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/3681/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3681/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3681/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3681/console

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2790/rebuild

review: Needs Fixing (continuous-integration)

[Kevin DuBois (kdub)]

> Hmm, by that definition a number or index of structures may use size_t. Just
> not a number of millimetres.

+1, also

+size_t mir_display_config_get_max_simultaneous_outputs(
On the deprecation path, probably doesn't need changes?: http://pastebin.ubuntu.com/23852554/

review: Needs Fixing

[lp:~mir-team/mir/size-t-display-config-api]

3965. By Brandon Schaefer on 2017-01-24

* mm -> int, %zu for portable printf

[Mir CI Bot (mir-ci-bot)]

FAILED: Continuous integration, rev:3965
https://mir-jenkins.ubuntu.com/job/mir-ci/2853/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/3758/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3837
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3827
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3827
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/3827
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/3785
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/3785/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3785/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/3785
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/3785/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3785/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3785/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3785/console

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2853/rebuild

review: Needs Fixing (continuous-integration)

[lp:~mir-team/mir/size-t-display-config-api]

3966. By Brandon Schaefer on 2017-01-24

* Missed lu -> zu

[Mir CI Bot (mir-ci-bot)]

PASSED: Continuous integration, rev:3966
https://mir-jenkins.ubuntu.com/job/mir-ci/2858/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/3766
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3845
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3835
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3835
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/3835
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/3793
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/3793/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3793
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3793/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/3793
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/3793/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3793
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3793/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3793
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3793/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3793
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3793/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2858/rebuild

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

I would personally prefer we used int for all this, since it's simpler to work with and our structures will never be big enough to overflow an int and require size_t. But getting over my personal preference, still needs fixing...

(4) Output ID is not an index or a size of something, so size_t is inappropriate. It's just an arbitrary unsigned int...

57 -int mir_output_get_id(MirOutput const* output);
58 +size_t mir_output_get_id(MirOutput const* output);

101 -int mir_output_get_id(MirOutput const* output)
102 +size_t mir_output_get_id(MirOutput const* output)

review: Needs Fixing

[Brandon Schaefer (brandontschaefer)]

I agree with one way or the other. Right now we have a mix of size_t and ints which causes casting on either end of the API usage. Maybe something we should discuss. Im inclined for size_t/unsigned because it enforces correct usage when accessing an array. No index should be negative, if we want to return an error we should have a check to return a bool.

And yeah get_id isnt a representative of the size of an object and should just be unsigned.

[lp:~mir-team/mir/size-t-display-config-api]

3967. By Brandon Schaefer on 2017-01-25

* Returned unsigned int vs size_t since id is not the size of an object

[Mir CI Bot (mir-ci-bot)]

FAILED: Continuous integration, rev:3967
https://mir-jenkins.ubuntu.com/job/mir-ci/2872/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/3787/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3866
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3856
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3856
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/3856
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/3814/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3814/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/3814/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3814
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3814/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3814
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3814/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3814
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3814/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2872/rebuild

review: Needs Fixing (continuous-integration)

[Alan Griffiths (alan-griffiths)]

> I would personally prefer we used int for all this, since it's simpler to work
> with and our structures will never be big enough to overflow an int and
> require size_t. But getting over my personal preference, still needs fixing...

+1

[Brandon Schaefer (brandontschaefer)]

Looks like the votes are in, and int being the main goal. Issue now is actually getting the params to int, as any function that takes an index is size_t already. This means already if you want to get num of outputs then use that as an index you'll need to cast.

Now to deprecate, would mean a new name there. So not sure if thats even wanted atm.

Unmerged revisions

3967. By Brandon Schaefer on 2017-01-25

* Returned unsigned int vs size_t since id is not the size of an object

3966. By Brandon Schaefer on 2017-01-24

* Missed lu -> zu

3965. By Brandon Schaefer on 2017-01-24

* mm -> int, %zu for portable printf

3964. By Brandon Schaefer on 2017-01-20

* Lets make our public API a bit better type wise. Indexes should never be negative so lets use unsigned

=== modified file 'examples/eglsquare.cpp'
--- examples/eglsquare.cpp	2017-01-18 02:29:37 +0000
+++ examples/eglsquare.cpp	2017-01-25 23:20:39 +0000
@@ -161,7 +161,7 @@
         unsigned int height{0};
         auto display_config = mir_connection_create_display_configuration(connection);
         auto num_outputs = mir_display_config_get_num_outputs(display_config);
-        for (auto i = 0; i < num_outputs; i++)
+        for (auto i = 0u; i < num_outputs; i++)
         {
             auto output = mir_display_config_get_output(display_config, i);
             auto state = mir_output_get_connection_state(output);

=== modified file 'include/client/mir_toolkit/mir_display_configuration.h'
--- include/client/mir_toolkit/mir_display_configuration.h	2017-01-23 22:35:37 +0000
+++ include/client/mir_toolkit/mir_display_configuration.h	2017-01-25 23:20:39 +0000
@@ -58,7 +58,7 @@
  * \returns      The maximum number of simultaneously active outputs
  *               supportable at this time.
  */
-int mir_display_config_get_max_simultaneous_outputs(
+size_t mir_display_config_get_max_simultaneous_outputs(
     MirDisplayConfig const* config);
 
 /**
@@ -77,7 +77,7 @@
  * \param [in]  config    The configuration to query
  * \returns      The number of outputs available in this configuration.
  */
-int mir_display_config_get_num_outputs(MirDisplayConfig const* config);
+size_t mir_display_config_get_num_outputs(MirDisplayConfig const* config);
 
 /**
  * Get a read-only handle to the index 'th output of this configuration
@@ -114,7 +114,7 @@
  * \param [in]  output  The MirOutput to query
  * \returns     The number of modes in the supported mode list of output.
  */
-int mir_output_get_num_modes(MirOutput const* output);
+size_t mir_output_get_num_modes(MirOutput const* output);
 
 /**
  * Get a handle for a mode descriptor from the list of supported modes.
@@ -206,7 +206,7 @@
  * \param [in]  output  The MirOutput to query
  * \returns     The number of pixel formats for output.
  */
-int mir_output_get_num_pixel_formats(MirOutput const* output);
+size_t mir_output_get_num_pixel_formats(MirOutput const* output);
 
 /**
  * Get a pixel format from the list of supported formats
@@ -247,7 +247,7 @@
  * \returns     The ID of output, which may be used to refer to it in other
  *              parts of the API.
  */
-int mir_output_get_id(MirOutput const* output);
+unsigned int mir_output_get_id(MirOutput const* output);
 
 /**
  * Get the physical connection type of an output.

=== modified file 'src/client/display_configuration_api.cpp'
--- src/client/display_configuration_api.cpp	2017-01-18 02:29:37 +0000
+++ src/client/display_configuration_api.cpp	2017-01-25 23:20:39 +0000
@@ -53,21 +53,21 @@
 }
 }
 
-int mir_display_config_get_num_outputs(MirDisplayConfig const* config)
+size_t mir_display_config_get_num_outputs(MirDisplayConfig const* config)
 {
     return config->display_output_size();
 }
 
 MirOutput const* mir_display_config_get_output(MirDisplayConfig const* config, size_t index)
 {
-    mir::require(index < static_cast<size_t>(mir_display_config_get_num_outputs(config)));
+    mir::require(index < mir_display_config_get_num_outputs(config));
 
     return output_to_client(&config->display_output(index));
 }
 
 MirOutput* mir_display_config_get_mutable_output(MirDisplayConfig* config, size_t index)
 {
-    mir::require(index < static_cast<size_t>(mir_display_config_get_num_outputs(config)));
+    mir::require(index < mir_display_config_get_num_outputs(config));
 
     return output_to_client(config->mutable_display_output(index));
 }
@@ -111,12 +111,12 @@
     return nullptr;
 }
 
-int mir_display_config_get_max_simultaneous_outputs(MirDisplayConfig const* config)
+size_t mir_display_config_get_max_simultaneous_outputs(MirDisplayConfig const* config)
 {
     return config->display_card(0).max_simultaneous_outputs();
 }
 
-int mir_output_get_id(MirOutput const* output)
+unsigned int mir_output_get_id(MirOutput const* output)
 {
     return output->output_id();
 }
@@ -141,14 +141,14 @@
     return output->physical_width_mm();
 }
 
-int mir_output_get_num_modes(MirOutput const* output)
+size_t mir_output_get_num_modes(MirOutput const* output)
 {
     return output->mode_size();
 }
 
 MirOutputMode const* mir_output_get_preferred_mode(MirOutput const* output)
 {
-    if (output->preferred_mode() >= static_cast<size_t>(mir_output_get_num_modes(output)))
+    if (output->preferred_mode() >= mir_output_get_num_modes(output))
     {
         return nullptr;
     }
@@ -160,7 +160,7 @@
 
 size_t mir_output_get_preferred_mode_index(MirOutput const* output)
 {
-    if (output->preferred_mode() >= static_cast<size_t>(mir_output_get_num_modes(output)))
+    if (output->preferred_mode() >= mir_output_get_num_modes(output))
     {
         return std::numeric_limits<size_t>::max();
     }
@@ -172,7 +172,7 @@
 
 MirOutputMode const* mir_output_get_current_mode(MirOutput const* output)
 {
-    if (output->current_mode() >= static_cast<size_t>(mir_output_get_num_modes(output)))
+    if (output->current_mode() >= mir_output_get_num_modes(output))
     {
         return nullptr;
     }
@@ -184,7 +184,7 @@
 
 size_t mir_output_get_current_mode_index(MirOutput const* output)
 {
-    if (output->current_mode() >= static_cast<size_t>(mir_output_get_num_modes(output)))
+    if (output->current_mode() >= mir_output_get_num_modes(output))
     {
         return std::numeric_limits<size_t>::max();
     }
@@ -199,8 +199,8 @@
 {
     auto mode = client_to_mode(client_mode);
 
-    int index = -1;
-    for (int i = 0; i < output->mode_size(); ++i)
+    size_t index = -1;
+    for (size_t i = 0; i < static_cast<size_t>(output->mode_size()); ++i)
     {
         if (mode->SerializeAsString() == output->mode(i).SerializeAsString())
         {
@@ -209,7 +209,6 @@
         }
     }
 
-    mir::require(index >= 0);
     mir::require(index < mir_output_get_num_modes(output));
 
     output->set_current_mode(static_cast<uint32_t>(index));
@@ -220,7 +219,7 @@
     return mode_to_client(&output->mode(index));
 }
 
-int mir_output_get_num_pixel_formats(MirOutput const* output)
+size_t mir_output_get_num_pixel_formats(MirOutput const* output)
 {
     return output->pixel_format_size();
 }

=== modified file 'src/utils/out.c'
--- src/utils/out.c	2017-01-18 02:29:37 +0000
+++ src/utils/out.c	2017-01-25 23:20:39 +0000
@@ -111,8 +111,8 @@
     {
         if (!strcmp(*action, "output"))
         {
-            int output_id;
-            if (++action < action_end && 1 == sscanf(*action, "%d", &output_id))
+            size_t output_id;
+            if (++action < action_end && 1 == sscanf(*action, "%zu", &output_id))
             {
                 targets = 0;
                 for (int i = 0; i < num_outputs; ++i)
@@ -410,7 +410,7 @@
 
         int num_outputs = mir_display_config_get_num_outputs(conf);
 
-        printf("Max %d simultaneous outputs\n",
+        printf("Max %zu simultaneous outputs\n",
                mir_display_config_get_max_simultaneous_outputs(conf));
 
         for (int i = 0; i < num_outputs; ++i)
@@ -419,7 +419,7 @@
             MirOutputConnectionState const state =
                 mir_output_get_connection_state(out);
 
-            printf("Output %d: %s, %s",
+            printf("Output %zu: %s, %s",
                    mir_output_get_id(out),
                    mir_output_type_name(mir_output_get_type(out)),
                    state_name(state));

=== modified file 'tests/acceptance-tests/test_new_display_configuration.cpp'
--- tests/acceptance-tests/test_new_display_configuration.cpp	2017-01-18 02:29:37 +0000
+++ tests/acceptance-tests/test_new_display_configuration.cpp	2017-01-25 23:20:39 +0000
@@ -413,7 +413,7 @@
 
     auto client_config = client.get_base_config();
 
-    for (int i = 0; i < mir_display_config_get_num_outputs(client_config.get()); ++i)
+    for (auto i = 0u; i < mir_display_config_get_num_outputs(client_config.get()); ++i)
     {
         auto output = mir_display_config_get_output(client_config.get(), i);
 
@@ -445,7 +445,7 @@
 
     auto client_config = client.get_base_config();
 
-    for (int i = 0; i < mir_display_config_get_num_outputs(client_config.get()); ++i)
+    for (auto i = 0u; i < mir_display_config_get_num_outputs(client_config.get()); ++i)
     {
         auto output = mir_display_config_get_output(client_config.get(), i);
 
@@ -485,7 +485,7 @@
 
     auto client_config = client.get_base_config();
 
-    for (int i = 0; i < mir_display_config_get_num_outputs(client_config.get()); ++i)
+    for (auto i = 0u; i < mir_display_config_get_num_outputs(client_config.get()); ++i)
     {
         auto output = mir_display_config_get_output(client_config.get(), i);
 
@@ -517,7 +517,7 @@
 
     auto client_config = client.get_base_config();
 
-    for (int i = 0; i < mir_display_config_get_num_outputs(client_config.get()); ++i)
+    for (auto i = 0u; i < mir_display_config_get_num_outputs(client_config.get()); ++i)
     {
         auto output = mir_display_config_get_output(client_config.get(), i);
 
@@ -609,7 +609,7 @@
 
     auto output = mir_display_config_get_output(config.get(), 0);
 
-    for (auto i = 0; i < mir_output_get_num_modes(output) ; ++i)
+    for (auto i = 0u; i < mir_output_get_num_modes(output) ; ++i)
     {
         auto mode = mir_output_get_mode(output, i);
         auto width = mir_output_mode_get_width(mode);
@@ -682,7 +682,7 @@
 
         auto output = mir_display_config_get_output(config.get(), 0);
 
-        for (auto i = 0; i < mir_output_get_num_modes(output) ; ++i)
+        for (auto i = 0u; i < mir_output_get_num_modes(output) ; ++i)
         {
             auto mode = mir_output_get_mode(output, i);
             auto width = mir_output_mode_get_width(mode);
@@ -750,7 +750,7 @@
         auto config = client.get_base_config();
 
         auto position = positions.begin();
-        for (auto i = 0; i < mir_display_config_get_num_outputs(config.get()); ++i)
+        for (auto i = 0u; i < mir_display_config_get_num_outputs(config.get()); ++i)
         {
             auto output = mir_display_config_get_output(config.get(), i);
 
@@ -802,7 +802,7 @@
     auto config = client.get_base_config();
 
     auto position = positions.begin();
-    for (auto i = 0; i < mir_display_config_get_num_outputs(config.get()); ++i)
+    for (auto i = 0u; i < mir_display_config_get_num_outputs(config.get()); ++i)
     {
         auto output = mir_display_config_get_output(config.get(), i);
 
@@ -851,7 +851,7 @@
 
     auto client_config = client.get_base_config();
 
-    for (int i = 0; i < mir_display_config_get_num_outputs(client_config.get()); ++i)
+    for (auto i = 0u; i < mir_display_config_get_num_outputs(client_config.get()); ++i)
     {
         auto output = mir_display_config_get_output(client_config.get(), i);
 
@@ -893,7 +893,7 @@
 
     auto client_config = client.get_base_config();
 
-    for (int i = 0; i < mir_display_config_get_num_outputs(client_config.get()); ++i)
+    for (auto i = 0u; i < mir_display_config_get_num_outputs(client_config.get()); ++i)
     {
         auto output = mir_display_config_get_output(client_config.get(), i);
 
@@ -939,7 +939,7 @@
 
     auto client_config = client.get_base_config();
 
-    for (int i = 0; i < mir_display_config_get_num_outputs(client_config.get()); ++i)
+    for (auto i = 0u; i < mir_display_config_get_num_outputs(client_config.get()); ++i)
     {
         auto output = mir_display_config_get_output(client_config.get(), i);
 
@@ -987,7 +987,7 @@
 
     auto client_config = client.get_base_config();
 
-    for (int i = 0; i < mir_display_config_get_num_outputs(client_config.get()); ++i)
+    for (auto i = 0u; i < mir_display_config_get_num_outputs(client_config.get()); ++i)
     {
         auto output = mir_display_config_get_mutable_output(client_config.get(), i);
 
@@ -1220,7 +1220,7 @@
 
     auto client_config = client.get_base_config();
 
-    for (int i = 0; i < mir_display_config_get_num_outputs(client_config.get()); ++i)
+    for (auto i = 0u; i < mir_display_config_get_num_outputs(client_config.get()); ++i)
     {
         auto output = mir_display_config_get_output(client_config.get(), i);
 
@@ -1246,7 +1246,7 @@
 
     auto client_config = client.get_base_config();
 
-    for (int i = 0; i < mir_display_config_get_num_outputs(client_config.get()); ++i)
+    for (auto i = 0u; i < mir_display_config_get_num_outputs(client_config.get()); ++i)
     {
         auto output = mir_display_config_get_output(client_config.get(), i);
 
@@ -1272,11 +1272,11 @@
 
     std::shared_ptr<MirDisplayConfig> config = client.get_base_config();
 
-    for (auto i = 0; i < mir_display_config_get_num_outputs(config.get()); ++i)
+    for (auto i = 0u; i < mir_display_config_get_num_outputs(config.get()); ++i)
     {
         auto output = mir_display_config_get_mutable_output(config.get(), i);
 
-        for (auto j = 0; j < mir_output_get_num_modes(output); ++j)
+        for (auto j = 0u; j < mir_output_get_num_modes(output); ++j)
         {
             auto mode = mir_output_get_mode(output, j);
 
@@ -1317,11 +1317,11 @@
     std::shared_ptr<MirDisplayConfig> old_config = client.get_base_config();
     std::shared_ptr<MirDisplayConfig> new_config = client.get_base_config();
 
-    for (auto i = 0; i < mir_display_config_get_num_outputs(new_config.get()); ++i)
+    for (auto i = 0u; i < mir_display_config_get_num_outputs(new_config.get()); ++i)
     {
         auto output = mir_display_config_get_mutable_output(new_config.get(), i);
 
-        for (auto j = 0; j < mir_output_get_num_modes(output); ++j)
+        for (auto j = 0u; j < mir_output_get_num_modes(output); ++j)
         {
             auto mode = mir_output_get_mode(output, j);
 
@@ -1381,11 +1381,11 @@
     std::shared_ptr<MirDisplayConfig> old_config = client.get_base_config();
     std::shared_ptr<MirDisplayConfig> new_config = client.get_base_config();
 
-    for (auto i = 0; i < mir_display_config_get_num_outputs(new_config.get()); ++i)
+    for (auto i = 0u; i < mir_display_config_get_num_outputs(new_config.get()); ++i)
     {
         auto output = mir_display_config_get_mutable_output(new_config.get(), i);
 
-        for (auto j = 0; j < mir_output_get_num_modes(output); ++j)
+        for (auto j = 0u; j < mir_output_get_num_modes(output); ++j)
         {
             auto mode = mir_output_get_mode(output, j);
 

=== modified file 'tests/mir_test/display_config_matchers.cpp'
--- tests/mir_test/display_config_matchers.cpp	2017-01-18 02:29:37 +0000
+++ tests/mir_test/display_config_matchers.cpp	2017-01-25 23:20:39 +0000
@@ -33,7 +33,7 @@
 
 size_t find_mode_index(MirOutput const* output, MirOutputMode const* mode)
 {
-    for (int i = 0; i < mir_output_get_num_modes(output); ++i)
+    for (size_t i = 0; i < mir_output_get_num_modes(output); ++i)
     {
         if (mir_output_get_mode(output, i) == mode)
         {
@@ -49,7 +49,7 @@
     TestDisplayConfiguration(mp::DisplayConfiguration const& protobuf_config)
     {
         /* Outputs */
-        for (int i = 0; i < protobuf_config.display_output_size(); i++)
+        for (size_t i = 0; i < static_cast<size_t>(protobuf_config.display_output_size()); i++)
         {
             auto const& protobuf_output = protobuf_config.display_output(i);
             mg::DisplayConfigurationOutput display_output
@@ -80,7 +80,7 @@
 
             /* Modes */
             std::vector<mg::DisplayConfigurationMode> modes;
-            for (int n = 0; n < protobuf_output.mode_size(); n++)
+            for (size_t n = 0; n < static_cast<size_t>(protobuf_output.mode_size()); n++)
             {
                 auto const& protobuf_mode = protobuf_output.mode(n);
                 modes.push_back(
@@ -94,7 +94,7 @@
 
             /* Pixel formats */
             std::vector<MirPixelFormat> pixel_formats;
-            for (int n = 0; n < protobuf_output.pixel_format_size(); n++)
+            for (size_t n = 0; n < static_cast<size_t>(protobuf_output.pixel_format_size()); n++)
             {
                 pixel_formats.push_back(
                     static_cast<MirPixelFormat>(protobuf_output.pixel_format(n)));
@@ -166,7 +166,7 @@
     TestDisplayConfiguration(MirDisplayConfig const* config)
     {
         /* Outputs */
-        for (int i = 0; i < mir_display_config_get_num_outputs(config); i++)
+        for (size_t i = 0; i < mir_display_config_get_num_outputs(config); i++)
         {
             auto const client_output = mir_display_config_get_output(config, i);
             mg::DisplayConfigurationOutput display_output
@@ -197,7 +197,7 @@
 
             /* Modes */
             std::vector<mg::DisplayConfigurationMode> modes;
-            for (int n = 0; n < mir_output_get_num_modes(client_output); n++)
+            for (size_t n = 0; n < mir_output_get_num_modes(client_output); n++)
             {
                 auto const client_mode = mir_output_get_mode(client_output, n);
                 modes.push_back(
@@ -212,7 +212,7 @@
 
             /* Pixel formats */
             std::vector<MirPixelFormat> pixel_formats;
-            for (int n = 0; n < mir_output_get_num_pixel_formats(client_output); n++)
+            for (size_t n = 0; n < mir_output_get_num_pixel_formats(client_output); n++)
             {
                 pixel_formats.push_back(mir_output_get_pixel_format(client_output, n));
             }