[Intel-gfx] [PATCH 2/2 i-g-t] lib/igt.cocci: Add 64-bit and float compare functions

Paulo Zanoni przanoni at gmail.com
Tue Jun 30 07:04:07 PDT 2015


2015-06-30 10:41 GMT-03:00 Michel Thierry <michel.thierry at intel.com>:
> Also include script output.
>
> Suggested-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> Signed-off-by: Michel Thierry <michel.thierry at intel.com>

I've asked a few times already whether igt.cocci is a requirement or
not, and nobody ever answered me. Can we please have an official
answer/discussion before we merge a patch running igt.cocci on the
whole codebase? See my previous emails for further arguments.

> ---
>  lib/igt.cocci                    | 42 +++++++++++++++++++++++++++++++
>  tests/gem_cs_tlb.c               |  2 +-
>  tests/kms_draw_crc.c             |  6 ++---
>  tests/kms_frontbuffer_tracking.c | 54 ++++++++++++++++++++--------------------
>  tests/kms_panel_fitting.c        |  2 +-
>  tests/kms_pipe_b_c_ivb.c         | 30 +++++++++++-----------
>  tests/kms_plane_scaling.c        |  2 +-
>  tests/kms_rotation_crc.c         |  4 +--
>  tests/pm_backlight.c             | 20 +++++++--------
>  9 files changed, 102 insertions(+), 60 deletions(-)
>
> diff --git a/lib/igt.cocci b/lib/igt.cocci
> index 3aee72f..f6b33fb 100644
> --- a/lib/igt.cocci
> +++ b/lib/igt.cocci
> @@ -174,6 +174,48 @@ int E3, E4;
>  + igt_assert_lt(E3, E4);
>  )
>
> +
> +// Use comparison macros instead of raw igt_assert when possible (double variables)
> +@@
> +double E1, E2;
> +@@
> +(
> +- igt_assert(E1 == E2);
> ++ igt_assert_eq_double(E1, E2);
> +)
> +
> +// Use comparison macros instead of raw igt_assert when possible (64-bit variables)
> +@@
> +typedef uint64_t;
> +uint64_t E1, E2;
> +long long E3, E4;
> +@@
> +(
> +- igt_assert(E1 == E2);
> ++ igt_assert_eq_u64(E1, E2);
> +|
> +- igt_assert(E1 != E2);
> ++ igt_assert_neq64(E1, E2);
> +|
> +- igt_assert(E1 <= E2);
> ++ igt_assert_lte64(E1, E2);
> +|
> +- igt_assert(E1 < E2);
> ++ igt_assert_lt64(E1, E2);
> +|
> +- igt_assert(E3 == E4);
> ++ igt_assert_eq64(E3, E4);
> +|
> +- igt_assert(E3 != E4);
> ++ igt_assert_neq64(E3, E4);
> +|
> +- igt_assert(E3 <= E4);
> ++ igt_assert_lte64(E3, E4);
> +|
> +- igt_assert(E3 < E4);
> ++ igt_assert_lt64(E3, E4);
> +)
> +
>  // avoid unused-result warnings when compiling with _FORTIFY_SOURCE defined
>  @@
>  identifier func =~ "^(read|write)$";
> diff --git a/tests/gem_cs_tlb.c b/tests/gem_cs_tlb.c
> index 62f446c..e4f6d28 100644
> --- a/tests/gem_cs_tlb.c
> +++ b/tests/gem_cs_tlb.c
> @@ -135,7 +135,7 @@ static void run_on_ring(int fd, unsigned ring_id, const char *ring_name)
>
>                 if (split > 0) {
>                         /* Check that we've managed to collide in the tlb. */
> -                       igt_assert(gtt_offset == gtt_offset_new);
> +                       igt_assert_eq_u64(gtt_offset, gtt_offset_new);
>
>                         /* We hang onto the storage of the old batch by keeping
>                          * the cpu mmap around. */
> diff --git a/tests/kms_draw_crc.c b/tests/kms_draw_crc.c
> index 9fcf997..88cee1e 100644
> --- a/tests/kms_draw_crc.c
> +++ b/tests/kms_draw_crc.c
> @@ -100,7 +100,7 @@ static void get_method_crc(enum igt_draw_method method, uint64_t tiling,
>
>         rc = drmModeSetCrtc(drm_fd, ms.crtc_id, fb.fb_id, 0, 0,
>                             &ms.connector_id, 1, ms.mode);
> -       igt_assert(rc == 0);
> +       igt_assert_eq(rc, 0);
>
>         igt_pipe_crc_collect_crc(pipe_crc, crc);
>
> @@ -141,7 +141,7 @@ static void get_fill_crc(uint64_t tiling, igt_crc_t *crc)
>
>         rc = drmModeSetCrtc(drm_fd, ms.crtc_id, fb.fb_id, 0, 0,
>                             &ms.connector_id, 1, ms.mode);
> -       igt_assert(rc == 0);
> +       igt_assert_eq(rc, 0);
>
>         igt_pipe_crc_collect_crc(pipe_crc, crc);
>
> @@ -167,7 +167,7 @@ static void fill_fb_subtest(void)
>
>         rc = drmModeSetCrtc(drm_fd, ms.crtc_id, fb.fb_id, 0, 0,
>                             &ms.connector_id, 1, ms.mode);
> -       igt_assert(rc == 0);
> +       igt_assert_eq(rc, 0);
>
>         igt_pipe_crc_collect_crc(pipe_crc, &base_crc);
>
> diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
> index 3785a5a..70cb5bc 100644
> --- a/tests/kms_frontbuffer_tracking.c
> +++ b/tests/kms_frontbuffer_tracking.c
> @@ -821,7 +821,7 @@ static struct rect pat4_get_rect(struct fb_region *fb, int r)
>  {
>         struct rect rect;
>
> -       igt_assert(r == 0);
> +       igt_assert_eq(r, 0);
>
>         rect.x = 0;
>         rect.y = 0;
> @@ -871,16 +871,16 @@ static void unset_all_crtcs(void)
>         for (i = 0; i < drm.res->count_crtcs; i++) {
>                 rc = drmModeSetCrtc(drm.fd, drm.res->crtcs[i], -1, 0, 0, NULL,
>                                     0, NULL);
> -               igt_assert(rc == 0);
> +               igt_assert_eq(rc, 0);
>
>                 rc = drmModeSetCursor(drm.fd, drm.res->crtcs[i], 0, 0, 0);
> -               igt_assert(rc == 0);
> +               igt_assert_eq(rc, 0);
>         }
>
>         for (i = 0; i < drm.planes->count_planes; i++) {
>                 rc = drmModeSetPlane(drm.fd, drm.planes->planes[i], 0, 0, 0, 0,
>                                      0, 0, 0, 0, 0, 0, 0);
> -               igt_assert(rc == 0);
> +               igt_assert_eq(rc, 0);
>         }
>  }
>
> @@ -914,7 +914,7 @@ static void start_busy_thread(struct igt_fb *fb)
>         busy_thread.height = fb->height;
>
>         rc = pthread_create(&busy_thread.thread, NULL, busy_thread_func, NULL);
> -       igt_assert(rc == 0);
> +       igt_assert_eq(rc, 0);
>  }
>
>  static void stop_busy_thread(void)
> @@ -967,7 +967,7 @@ static void init_blue_crc(void)
>         rc = drmModeSetCrtc(drm.fd, prim_mode_params.crtc_id,
>                             blue.fb_id, 0, 0, &prim_mode_params.connector_id, 1,
>                             prim_mode_params.mode);
> -       igt_assert(rc == 0);
> +       igt_assert_eq(rc, 0);
>         collect_crcs(&blue_crc);
>
>         print_crc("Blue CRC:  ", &blue_crc);
> @@ -1010,7 +1010,7 @@ static void init_crcs(struct draw_pattern_info *pattern)
>                                    tmp_fbs[r].fb_id, 0, 0,
>                                    &prim_mode_params.connector_id, 1,
>                                    prim_mode_params.mode);
> -               igt_assert(rc == 0);
> +               igt_assert_eq(rc, 0);
>                 collect_crcs(&pattern->crcs[r]);
>         }
>
> @@ -1109,7 +1109,7 @@ static void setup_sink_crc(void)
>         set_mode_for_params(&prim_mode_params);
>
>         sink_crc.fd = igt_debugfs_open("i915_sink_crc_eDP1", O_RDONLY);
> -       igt_assert(sink_crc.fd >= 0);
> +       igt_assert_lte(0, sink_crc.fd);
>
>         rc = read(sink_crc.fd, crc.data, SINK_CRC_SIZE);
>         errno_ = errno;
> @@ -1184,7 +1184,7 @@ static bool fbc_supported_on_chipset(void)
>  static void setup_fbc(void)
>  {
>         fbc.fd = igt_debugfs_open("i915_fbc_status", O_RDONLY);
> -       igt_assert(fbc.fd >= 0);
> +       igt_assert_lte(0, fbc.fd);
>
>         if (!fbc_supported_on_chipset()) {
>                 igt_info("Can't test FBC: not supported on this chipset\n");
> @@ -1220,7 +1220,7 @@ static void setup_psr(void)
>         }
>
>         psr.fd = igt_debugfs_open("i915_edp_psr_status", O_RDONLY);
> -       igt_assert(psr.fd >= 0);
> +       igt_assert_lte(0, psr.fd);
>
>         if (!psr_sink_has_support()) {
>                 igt_info("Can't test PSR: not supported by sink.\n");
> @@ -1426,13 +1426,13 @@ static void set_cursor_for_test(const struct test_mode *t,
>         fill_fb_region(&params->cursor, 0xFF0000FF);
>
>         rc = drmModeMoveCursor(drm.fd, params->crtc_id, 0, 0);
> -       igt_assert(rc == 0);
> +       igt_assert_eq(rc, 0);
>
>         rc = drmModeSetCursor(drm.fd, params->crtc_id,
>                               params->cursor.fb->gem_handle,
>                               params->cursor.w,
>                               params->cursor.h);
> -       igt_assert(rc == 0);
> +       igt_assert_eq(rc, 0);
>
>         do_assertions(ASSERT_NO_ACTION_CHANGE);
>  }
> @@ -1449,7 +1449,7 @@ static void set_sprite_for_test(const struct test_mode *t,
>                              params->sprite.w, params->sprite.h,
>                              0, 0, params->sprite.w << 16,
>                              params->sprite.h << 16);
> -       igt_assert(rc == 0);
> +       igt_assert_eq(rc, 0);
>
>         do_assertions(ASSERT_NO_ACTION_CHANGE);
>  }
> @@ -1830,7 +1830,7 @@ static void flip_subtest(const struct test_mode *t)
>
>                 rc = drmModePageFlip(drm.fd, params->crtc_id,
>                                      params->fb.fb->fb_id, 0, NULL);
> -               igt_assert(rc == 0);
> +               igt_assert_eq(rc, 0);
>
>                 do_assertions(assertions);
>         }
> @@ -1875,7 +1875,7 @@ static void move_subtest(const struct test_mode *t)
>                 case PLANE_CUR:
>                         rc = drmModeMoveCursor(drm.fd, params->crtc_id, rect.x,
>                                                rect.y);
> -                       igt_assert(rc == 0);
> +                       igt_assert_eq(rc, 0);
>                         break;
>                 case PLANE_SPR:
>                         rc = drmModeSetPlane(drm.fd, params->sprite_id,
> @@ -1884,7 +1884,7 @@ static void move_subtest(const struct test_mode *t)
>                                              rect.x, rect.y, rect.w,
>                                              rect.h, 0, 0, rect.w << 16,
>                                              rect.h << 16);
> -                       igt_assert(rc == 0);
> +                       igt_assert_eq(rc, 0);
>                         break;
>                 default:
>                         igt_assert(false);
> @@ -1936,13 +1936,13 @@ static void onoff_subtest(const struct test_mode *t)
>                         case PLANE_CUR:
>                                 rc = drmModeSetCursor(drm.fd, params->crtc_id,
>                                                       0, 0, 0);
> -                               igt_assert(rc == 0);
> +                               igt_assert_eq(rc, 0);
>                                 break;
>                         case PLANE_SPR:
>                                 rc = drmModeSetPlane(drm.fd, params->sprite_id,
>                                                      0, 0, 0, 0, 0, 0, 0, 0, 0,
>                                                      0, 0);
> -                               igt_assert(rc == 0);
> +                               igt_assert_eq(rc, 0);
>                                 break;
>                         default:
>                                 igt_assert(false);
> @@ -1956,7 +1956,7 @@ static void onoff_subtest(const struct test_mode *t)
>                                                   params->cursor.fb->gem_handle,
>                                                   params->cursor.w,
>                                                   params->cursor.h);
> -                               igt_assert(rc == 0);
> +                               igt_assert_eq(rc, 0);
>                                 break;
>                         case PLANE_SPR:
>                                 rc = drmModeSetPlane(drm.fd, params->sprite_id,
> @@ -1967,7 +1967,7 @@ static void onoff_subtest(const struct test_mode *t)
>                                                      0,
>                                                      params->sprite.w << 16,
>                                                      params->sprite.h << 16);
> -                               igt_assert(rc == 0);
> +                               igt_assert_eq(rc, 0);
>                                 break;
>                         default:
>                                 igt_assert(false);
> @@ -2014,7 +2014,7 @@ static void fullscreen_plane_subtest(const struct test_mode *t)
>                              fullscreen_fb.height, 0, 0,
>                              fullscreen_fb.width << 16,
>                              fullscreen_fb.height << 16);
> -       igt_assert(rc == 0);
> +       igt_assert_eq(rc, 0);
>         update_wanted_crc(t, &pattern->crcs[0]);
>
>         switch (t->screen) {
> @@ -2032,7 +2032,7 @@ static void fullscreen_plane_subtest(const struct test_mode *t)
>
>         rc = drmModeSetPlane(drm.fd, params->sprite_id, 0, 0, 0, 0, 0, 0, 0, 0,
>                              0, 0, 0);
> -       igt_assert(rc == 0);
> +       igt_assert_eq(rc, 0);
>
>         if (t->screen == SCREEN_PRIM)
>                 assertions = ASSERT_LAST_ACTION_CHANGED;
> @@ -2114,23 +2114,23 @@ static int opt_handler(int option, int option_index, void *data)
>                 opt.step++;
>                 break;
>         case 'n':
> -               igt_assert(opt.only_feature == FEATURE_COUNT);
> +               igt_assert_eq(opt.only_feature, FEATURE_COUNT);
>                 opt.only_feature = FEATURE_NONE;
>                 break;
>         case 'f':
> -               igt_assert(opt.only_feature == FEATURE_COUNT);
> +               igt_assert_eq(opt.only_feature, FEATURE_COUNT);
>                 opt.only_feature = FEATURE_FBC;
>                 break;
>         case 'p':
> -               igt_assert(opt.only_feature == FEATURE_COUNT);
> +               igt_assert_eq(opt.only_feature, FEATURE_COUNT);
>                 opt.only_feature = FEATURE_PSR;
>                 break;
>         case '1':
> -               igt_assert(opt.only_pipes == PIPE_COUNT);
> +               igt_assert_eq(opt.only_pipes, PIPE_COUNT);
>                 opt.only_pipes = PIPE_SINGLE;
>                 break;
>         case '2':
> -               igt_assert(opt.only_pipes == PIPE_COUNT);
> +               igt_assert_eq(opt.only_pipes, PIPE_COUNT);
>                 opt.only_pipes = PIPE_DUAL;
>                 break;
>         default:
> diff --git a/tests/kms_panel_fitting.c b/tests/kms_panel_fitting.c
> index 2d079b4..4e789dc 100644
> --- a/tests/kms_panel_fitting.c
> +++ b/tests/kms_panel_fitting.c
> @@ -127,7 +127,7 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
>                                 &output->id,
>                                 1,
>                                 mode);
> -               igt_assert(ret == 0);
> +               igt_assert_eq(ret, 0);
>         } else {
>                 igt_display_commit2(display, s);
>         }
> diff --git a/tests/kms_pipe_b_c_ivb.c b/tests/kms_pipe_b_c_ivb.c
> index b6e234c..9a0a336 100644
> --- a/tests/kms_pipe_b_c_ivb.c
> +++ b/tests/kms_pipe_b_c_ivb.c
> @@ -96,7 +96,7 @@ set_mode_on_pipe(data_t *data, enum pipe pipe, igt_output_t *output)
>                                     mode->hdisplay, mode->vdisplay,
>                                     DRM_FORMAT_XRGB8888, I915_TILING_NONE,
>                                     1.0, 1.0, 1.0, &fb);
> -       igt_assert(fb_id >= 0);
> +       igt_assert_lte(0, fb_id);
>
>         igt_plane_set_fb(primary, &fb);
>         return igt_display_try_commit2(&data->display, COMMIT_LEGACY);
> @@ -152,12 +152,12 @@ test_dpms(data_t *data)
>                  kmstest_pipe_name(PIPE_C), igt_output_name(output2));
>
>         ret = set_big_mode_on_pipe(data, PIPE_B, output1);
> -       igt_assert(ret == 0);
> +       igt_assert_eq(ret, 0);
>
>         kmstest_set_connector_dpms(data->drm_fd, output1->config.connector, DRM_MODE_DPMS_OFF);
>
>         ret = set_big_mode_on_pipe(data, PIPE_C, output2);
> -       igt_assert(ret != 0);
> +       igt_assert_neq(ret, 0);
>  }
>
>  static void
> @@ -174,13 +174,13 @@ test_lane_reduction(data_t *data)
>                  kmstest_pipe_name(PIPE_C), igt_output_name(output2));
>
>         ret = set_big_mode_on_pipe(data, PIPE_B, output1);
> -       igt_assert(ret == 0);
> +       igt_assert_eq(ret, 0);
>
>         ret = set_normal_mode_on_pipe(data, PIPE_B, output1);
> -       igt_assert(ret == 0);
> +       igt_assert_eq(ret, 0);
>
>         ret = set_normal_mode_on_pipe(data, PIPE_C, output2);
> -       igt_assert(ret == 0);
> +       igt_assert_eq(ret, 0);
>  }
>
>  static void
> @@ -197,16 +197,16 @@ test_disable_pipe_B(data_t *data)
>                  kmstest_pipe_name(PIPE_C), igt_output_name(output2));
>
>         ret = set_big_mode_on_pipe(data, PIPE_B, output1);
> -       igt_assert(ret == 0);
> +       igt_assert_eq(ret, 0);
>
>         ret = disable_pipe(data, PIPE_B, output1);
> -       igt_assert(ret == 0);
> +       igt_assert_eq(ret, 0);
>
>         ret = set_normal_mode_on_pipe(data, PIPE_C, output2);
> -       igt_assert(ret == 0);
> +       igt_assert_eq(ret, 0);
>
>         ret = set_normal_mode_on_pipe(data, PIPE_B, output1);
> -       igt_assert(ret == 0);
> +       igt_assert_eq(ret, 0);
>  }
>
>  static void
> @@ -223,13 +223,13 @@ test_from_C_to_B_with_3_lanes(data_t *data)
>                  kmstest_pipe_name(PIPE_C), igt_output_name(output2));
>
>         ret = set_normal_mode_on_pipe(data, PIPE_C, output2);
> -       igt_assert(ret == 0);
> +       igt_assert_eq(ret, 0);
>
>         ret = disable_pipe(data, PIPE_C, output2);
> -       igt_assert(ret == 0);
> +       igt_assert_eq(ret, 0);
>
>         ret = set_big_mode_on_pipe(data, PIPE_B, output1);
> -       igt_assert(ret == 0);
> +       igt_assert_eq(ret, 0);
>  }
>
>  static void
> @@ -246,10 +246,10 @@ test_fail_enable_pipe_C_while_B_has_3_lanes(data_t *data)
>                  kmstest_pipe_name(PIPE_C), igt_output_name(output2));
>
>         ret = set_big_mode_on_pipe(data, PIPE_B, output1);
> -       igt_assert(ret == 0);
> +       igt_assert_eq(ret, 0);
>
>         ret = set_normal_mode_on_pipe(data, PIPE_C, output2);
> -       igt_assert(ret != 0);
> +       igt_assert_neq(ret, 0);
>  }
>
>  static data_t data;
> diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
> index 00db5cb..77c57ab 100644
> --- a/tests/kms_plane_scaling.c
> +++ b/tests/kms_plane_scaling.c
> @@ -129,7 +129,7 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
>                                 &output->id,
>                                 1,
>                                 mode);
> -               igt_assert(ret == 0);
> +               igt_assert_eq(ret, 0);
>         } else {
>                 igt_display_commit2(display, s);
>         }
> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> index 3fd77c4..c61c3f8 100644
> --- a/tests/kms_rotation_crc.c
> +++ b/tests/kms_rotation_crc.c
> @@ -240,9 +240,9 @@ static void test_plane_rotation(data_t *data, enum igt_plane plane_type)
>                         igt_plane_set_rotation(plane, data->rotation);
>                         ret = igt_display_try_commit2(display, commit);
>                         if (data->override_fmt || data->override_tiling) {
> -                               igt_assert(ret == -EINVAL);
> +                               igt_assert_eq(ret, -EINVAL);
>                         } else {
> -                               igt_assert(ret == 0);
> +                               igt_assert_eq(ret, 0);
>                                 igt_pipe_crc_collect_crc(data->pipe_crc,
>                                                          &crc_output);
>                                 igt_assert_crc_equal(&data->ref_crc,
> diff --git a/tests/pm_backlight.c b/tests/pm_backlight.c
> index d02336d..136e127 100644
> --- a/tests/pm_backlight.c
> +++ b/tests/pm_backlight.c
> @@ -95,12 +95,12 @@ static void test_and_verify(int val)
>  {
>         int result;
>
> -       igt_assert(backlight_write(val, "brightness") == 0);
> -       igt_assert(backlight_read(&result, "brightness") == 0);
> +       igt_assert_eq(backlight_write(val, "brightness"), 0);
> +       igt_assert_eq(backlight_read(&result, "brightness"), 0);
>         /* Check that the exact value sticks */
> -       igt_assert(result == val);
> +       igt_assert_eq(result, val);
>
> -       igt_assert(backlight_read(&result, "actual_brightness") == 0);
> +       igt_assert_eq(backlight_read(&result, "actual_brightness"), 0);
>         /* Some rounding may happen depending on hw. Just check that it's close enough. */
>         igt_assert(result <= val + val * TOLERANCE / 100 && result >= val - val * TOLERANCE / 100);
>  }
> @@ -118,15 +118,15 @@ static void test_bad_brightness(int max)
>         /* First write some sane value */
>         backlight_write(max / 2, "brightness");
>         /* Writing invalid values should fail and not change the value */
> -       igt_assert(backlight_write(-1, "brightness") < 0);
> +       igt_assert_lt(backlight_write(-1, "brightness"), 0);
>         backlight_read(&val, "brightness");
> -       igt_assert(val == max / 2);
> -       igt_assert(backlight_write(max + 1, "brightness") < 0);
> +       igt_assert_eq(val, max / 2);
> +       igt_assert_lt(backlight_write(max + 1, "brightness"), 0);
>         backlight_read(&val, "brightness");
> -       igt_assert(val == max / 2);
> -       igt_assert(backlight_write(INT_MAX, "brightness") < 0);
> +       igt_assert_eq(val, max / 2);
> +       igt_assert_lt(backlight_write(INT_MAX, "brightness"), 0);
>         backlight_read(&val, "brightness");
> -       igt_assert(val == max / 2);
> +       igt_assert_eq(val, max / 2);
>  }
>
>  static void test_fade(int max)
> --
> 2.4.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni


More information about the Intel-gfx mailing list