On Tue, Jul 21, 2015 at 02:08:23PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>
> A helpful function for when you want to read a whole debugfs file to a
> string and don't want to worry about opening and closing file
> descriptors and asserting buffer sizes.
>
> We've been using this already for kms_frontbuffer_tracking and
> kms_fbcon_fbt, so the only test with new code here is kms_fbc_crc.
>
> Also notice that for kms_fbc_crc we had to increase the buffer size
> since the file can sometimes be bigger than 64 bytes - depending on
> the reason why FBC is disabled.
>
> Of course, there are probably many other programs we can patch, but
> I'm not doing this now.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> ---
> lib/igt_debugfs.c | 27 +++++++++++++++++++++++++++
> lib/igt_debugfs.h | 1 +
> tests/kms_fbc_crc.c | 18 ++++--------------
> tests/kms_fbcon_fbt.c | 25 ++++---------------------
> tests/kms_frontbuffer_tracking.c | 33 ++++++++-------------------------
> 5 files changed, 44 insertions(+), 60 deletions(-)
>
> diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> index a2c6fe2..bc1bb05 100644
> --- a/lib/igt_debugfs.c
> +++ b/lib/igt_debugfs.c
> @@ -185,6 +185,33 @@ FILE *igt_debugfs_fopen(const char *filename,
> return fopen(buf, mode);
> }
>
> +/**
> + * igt_debugfs_read:
> + * @filename: file name
> + * @buf: buffer where the contents will be stored, allocated by the caller
> + * @buf_size: size of the buffer
> + *
> + * This function opens the debugfs file, reads it, stores the content in the
> + * provided buffer, then closes the file. Users should make sure that the buffer
> + * provided is big enough to fit the whole file, plus one byte.
Hm if we want a helper function to slurp in an entire file I'd go with one
that mallocs a suitably big buffer. That means more free calls, but imo
that's less onerous than getting buffer sizes right (well at least for a
library function).
If you don't like that I'd rename this with a __ prefix and do a
#define igt_debugfs_read(file, buf) \
__igt_debugfs_read((file), (buf), sizeof(buf))
That way it's fairly impossible to get the static sizing wrong at least.
-Daniel
> + */
> +void igt_debugfs_read(const char *filename, char *buf, int buf_size)
> +{
> + FILE *file;
> + size_t n_read;
> +
> + file = igt_debugfs_fopen(filename, "r");
> + igt_assert(file);
> +
> + n_read = fread(buf, 1, buf_size - 1, file);
> + igt_assert(n_read > 0);
> + igt_assert(feof(file));
> +
> + buf[n_read] = '\0';
> +
> + igt_assert(fclose(file) == 0);
> +}
> +
> /*
> * Pipe CRC
> */
> diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
> index ece7148..a0bb75e 100644
> --- a/lib/igt_debugfs.h
> +++ b/lib/igt_debugfs.h
> @@ -34,6 +34,7 @@ enum pipe;
> int igt_debugfs_open(const char *filename, int mode);
> FILE *igt_debugfs_fopen(const char *filename,
> const char *mode);
> +void igt_debugfs_read(const char *filename, char *buf, int buf_size);
>
> /*
> * Pipe CRC
> diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
> index f2a86a6..28cc165 100644
> --- a/tests/kms_fbc_crc.c
> +++ b/tests/kms_fbc_crc.c
> @@ -215,14 +215,9 @@ static void fill_mmap_gtt(data_t *data, uint32_t handle, unsigned char color)
>
> static bool fbc_enabled(data_t *data)
> {
> - FILE *status;
> - char str[64] = {};
> + char str[128] = {};
>
> - status = igt_debugfs_fopen("i915_fbc_status", "r");
> - igt_assert(status);
> -
> - igt_assert(fread(str, 1, sizeof(str) - 1, status) > 0);
> - fclose(status);
> + igt_debugfs_read("i915_fbc_status", str, sizeof(str));
> return strstr(str, "FBC enabled") != NULL;
> }
>
> @@ -544,8 +539,7 @@ igt_main
> igt_skip_on_simulation();
>
> igt_fixture {
> - char buf[64];
> - FILE *status;
> + char buf[128];
>
> data.drm_fd = drm_open_any_master();
> kmstest_set_vt_graphics_mode();
> @@ -554,11 +548,7 @@ igt_main
>
> igt_require_pipe_crc();
>
> - status = igt_debugfs_fopen("i915_fbc_status", "r");
> - igt_require_f(status, "No i915_fbc_status found\n");
> - igt_assert_lt(0, fread(buf, 1, sizeof(buf), status));
> - fclose(status);
> - buf[sizeof(buf) - 1] = '\0';
> + igt_debugfs_read("i915_fbc_status", buf, sizeof(buf));
> igt_require_f(!strstr(buf, "unsupported on this chipset"),
> "FBC not supported\n");
>
> diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
> index f075014..c201fde 100644
> --- a/tests/kms_fbcon_fbt.c
> +++ b/tests/kms_fbcon_fbt.c
> @@ -86,28 +86,11 @@ static void teardown_drm(struct drm_info *drm)
> igt_assert(close(drm->fd) == 0);
> }
>
> -static void debugfs_read(const char *filename, char *buf, int buf_size)
> -{
> - FILE *file;
> - size_t n_read;
> -
> - file = igt_debugfs_fopen(filename, "r");
> - igt_assert(file);
> -
> - n_read = fread(buf, 1, buf_size - 1, file);
> - igt_assert(n_read > 0);
> - igt_assert(feof(file));
> -
> - buf[n_read] = '\0';
> -
> - igt_assert(fclose(file) == 0);
> -}
> -
> static bool fbc_supported_on_chipset(void)
> {
> char buf[128];
>
> - debugfs_read("i915_fbc_status", buf, ARRAY_SIZE(buf));
> + igt_debugfs_read("i915_fbc_status", buf, ARRAY_SIZE(buf));
> return !strstr(buf, "FBC unsupported on this chipset\n");
> }
>
> @@ -120,7 +103,7 @@ static bool fbc_is_enabled(void)
> {
> char buf[128];
>
> - debugfs_read("i915_fbc_status", buf, ARRAY_SIZE(buf));
> + igt_debugfs_read("i915_fbc_status", buf, ARRAY_SIZE(buf));
> return strstr(buf, "FBC enabled\n");
> }
>
> @@ -172,7 +155,7 @@ static bool psr_supported_on_chipset(void)
> {
> char buf[256];
>
> - debugfs_read("i915_edp_psr_status", buf, ARRAY_SIZE(buf));
> + igt_debugfs_read("i915_edp_psr_status", buf, ARRAY_SIZE(buf));
> return strstr(buf, "Sink_Support: yes\n");
> }
>
> @@ -185,7 +168,7 @@ static bool psr_is_enabled(void)
> {
> char buf[256];
>
> - debugfs_read("i915_edp_psr_status", buf, ARRAY_SIZE(buf));
> + igt_debugfs_read("i915_edp_psr_status", buf, ARRAY_SIZE(buf));
> return strstr(buf, "\nActive: yes\n");
> }
>
> diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
> index e9be045..e162e91 100644
> --- a/tests/kms_frontbuffer_tracking.c
> +++ b/tests/kms_frontbuffer_tracking.c
> @@ -556,28 +556,11 @@ static bool set_mode_for_params(struct modeset_params *params)
> return (rc == 0);
> }
>
> -static void debugfs_read(const char *filename, char *buf, int buf_size)
> -{
> - FILE *file;
> - size_t n_read;
> -
> - file = igt_debugfs_fopen(filename, "r");
> - igt_assert(file);
> -
> - n_read = fread(buf, 1, buf_size - 1, file);
> - igt_assert(n_read > 0);
> - igt_assert(feof(file));
> -
> - buf[n_read] = '\0';
> -
> - igt_assert(fclose(file) == 0);
> -}
> -
> static bool fbc_is_enabled(void)
> {
> char buf[128];
>
> - debugfs_read("i915_fbc_status", buf, ARRAY_SIZE(buf));
> + igt_debugfs_read("i915_fbc_status", buf, ARRAY_SIZE(buf));
> return strstr(buf, "FBC enabled\n");
> }
>
> @@ -585,7 +568,7 @@ static bool psr_is_enabled(void)
> {
> char buf[256];
>
> - debugfs_read("i915_edp_psr_status", buf, ARRAY_SIZE(buf));
> + igt_debugfs_read("i915_edp_psr_status", buf, ARRAY_SIZE(buf));
> return (strstr(buf, "\nActive: yes\n"));
> }
>
> @@ -596,7 +579,7 @@ static struct timespec fbc_get_last_action(void)
> char *action;
> ssize_t n_read;
>
> - debugfs_read("i915_fbc_status", buf, ARRAY_SIZE(buf));
> + igt_debugfs_read("i915_fbc_status", buf, ARRAY_SIZE(buf));
>
> action = strstr(buf, "\nLast action:");
> igt_assert(action);
> @@ -645,7 +628,7 @@ static void fbc_setup_last_action(void)
> char buf[128];
> char *action;
>
> - debugfs_read("i915_fbc_status", buf, ARRAY_SIZE(buf));
> + igt_debugfs_read("i915_fbc_status", buf, ARRAY_SIZE(buf));
>
> action = strstr(buf, "\nLast action:");
> if (!action) {
> @@ -664,7 +647,7 @@ static bool fbc_is_compressing(void)
> {
> char buf[128];
>
> - debugfs_read("i915_fbc_status", buf, ARRAY_SIZE(buf));
> + igt_debugfs_read("i915_fbc_status", buf, ARRAY_SIZE(buf));
> return strstr(buf, "\nCompressing: yes\n") != NULL;
> }
>
> @@ -677,7 +660,7 @@ static void fbc_setup_compressing(void)
> {
> char buf[128];
>
> - debugfs_read("i915_fbc_status", buf, ARRAY_SIZE(buf));
> + igt_debugfs_read("i915_fbc_status", buf, ARRAY_SIZE(buf));
>
> if (strstr(buf, "\nCompressing:"))
> fbc.supports_compressing = true;
> @@ -1187,7 +1170,7 @@ static bool fbc_supported_on_chipset(void)
> {
> char buf[128];
>
> - debugfs_read("i915_fbc_status", buf, ARRAY_SIZE(buf));
> + igt_debugfs_read("i915_fbc_status", buf, ARRAY_SIZE(buf));
> return !strstr(buf, "FBC unsupported on this chipset\n");
> }
>
> @@ -1211,7 +1194,7 @@ static bool psr_sink_has_support(void)
> {
> char buf[256];
>
> - debugfs_read("i915_edp_psr_status", buf, ARRAY_SIZE(buf));
> + igt_debugfs_read("i915_edp_psr_status", buf, ARRAY_SIZE(buf));
> return strstr(buf, "Sink_Support: yes\n");
> }
>
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
