diff mbox series

[V2] arb_texture_view: fix security format warnings

Message ID 20181130155051.9249-1-ross.burton@intel.com
State Accepted
Commit 379d5a1f73b1cc50614eb624038fc508686ec88d
Headers show
Series [V2] arb_texture_view: fix security format warnings | expand

Commit Message

Ross Burton Nov. 30, 2018, 3:50 p.m. UTC
If built with -Werror=format-security then Piglit fails to build:

/tests/spec/arb_texture_view/rendering-layers-image.c:150:8:
error: format not a string literal and no format arguments [-Werror=format-security]
         (desc)); \
         ^~~~~~

In this case test->uniform_type is being turned into a string using snprintf()
and then passed to piglit_report_subtest_result() which takes a format string,
but GCC can't verify the format.

As _subtest_report() takes a format string, we can just remove the snprintf()
and let it construct the label.

Also as X is used once and doesn't make the code clearer, just inline it.

Signed-off-by: Ross Burton <ross.burton@intel.com>
---
 tests/spec/arb_texture_view/rendering-layers-image.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

Comments

Eric Engestrom Nov. 30, 2018, 5:37 p.m. UTC | #1
On Friday, 2018-11-30 15:50:51 +0000, Ross Burton wrote:
> If built with -Werror=format-security then Piglit fails to build:
> 
> /tests/spec/arb_texture_view/rendering-layers-image.c:150:8:
> error: format not a string literal and no format arguments [-Werror=format-security]
>          (desc)); \
>          ^~~~~~
> 
> In this case test->uniform_type is being turned into a string using snprintf()
> and then passed to piglit_report_subtest_result() which takes a format string,
> but GCC can't verify the format.
> 
> As _subtest_report() takes a format string, we can just remove the snprintf()
> and let it construct the label.
> 
> Also as X is used once and doesn't make the code clearer, just inline it.
> 
> Signed-off-by: Ross Burton <ross.burton@intel.com>

Thanks :)
Reviewed-by: Eric Engestrom <eric.engestrom@intel.com>

> ---
>  tests/spec/arb_texture_view/rendering-layers-image.c | 19 ++++++-------------
>  1 file changed, 6 insertions(+), 13 deletions(-)
> 
> diff --git a/tests/spec/arb_texture_view/rendering-layers-image.c b/tests/spec/arb_texture_view/rendering-layers-image.c
> index 415b01657..86148075b 100644
> --- a/tests/spec/arb_texture_view/rendering-layers-image.c
> +++ b/tests/spec/arb_texture_view/rendering-layers-image.c
> @@ -142,26 +142,19 @@ test_render_layers(const struct test_info *test)
>  	return pass;
>  }
>  
> -#define X(f, desc) \
> -	do { \
> -		const bool subtest_pass = (f); \
> -		piglit_report_subtest_result(subtest_pass \
> -						 ? PIGLIT_PASS : PIGLIT_FAIL, \
> -						 (desc)); \
> -		pass = pass && subtest_pass; \
> -	} while (0)
> -
>  enum piglit_result
>  piglit_display(void)
>  {
>  	bool pass = true;
>  	for (int test_idx = 0; test_idx < ARRAY_SIZE(tests); test_idx++) {
>  		const struct test_info *test = &tests[test_idx];
> -		char test_name[128];
> -		snprintf(test_name, sizeof(test_name), "layers rendering of %s", test->uniform_type);
> -		X(test_render_layers(test), test_name);
> +
> +		const bool subtest_pass = test_render_layers(test);
> +
> +		piglit_report_subtest_result(subtest_pass ? PIGLIT_PASS : PIGLIT_FAIL,
> +					     "layers rendering of %s", test->uniform_type);
> +		pass = pass && subtest_pass;
>  	}
> -#undef X
>  	pass = piglit_check_gl_error(GL_NO_ERROR) && pass;
>  	return pass ? PIGLIT_PASS : PIGLIT_FAIL;
>  }
> -- 
> 2.11.0
> 
> _______________________________________________
> Piglit mailing list
> Piglit@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
diff mbox series

Patch

diff --git a/tests/spec/arb_texture_view/rendering-layers-image.c b/tests/spec/arb_texture_view/rendering-layers-image.c
index 415b01657..86148075b 100644
--- a/tests/spec/arb_texture_view/rendering-layers-image.c
+++ b/tests/spec/arb_texture_view/rendering-layers-image.c
@@ -142,26 +142,19 @@  test_render_layers(const struct test_info *test)
 	return pass;
 }
 
-#define X(f, desc) \
-	do { \
-		const bool subtest_pass = (f); \
-		piglit_report_subtest_result(subtest_pass \
-						 ? PIGLIT_PASS : PIGLIT_FAIL, \
-						 (desc)); \
-		pass = pass && subtest_pass; \
-	} while (0)
-
 enum piglit_result
 piglit_display(void)
 {
 	bool pass = true;
 	for (int test_idx = 0; test_idx < ARRAY_SIZE(tests); test_idx++) {
 		const struct test_info *test = &tests[test_idx];
-		char test_name[128];
-		snprintf(test_name, sizeof(test_name), "layers rendering of %s", test->uniform_type);
-		X(test_render_layers(test), test_name);
+
+		const bool subtest_pass = test_render_layers(test);
+
+		piglit_report_subtest_result(subtest_pass ? PIGLIT_PASS : PIGLIT_FAIL,
+					     "layers rendering of %s", test->uniform_type);
+		pass = pass && subtest_pass;
 	}
-#undef X
 	pass = piglit_check_gl_error(GL_NO_ERROR) && pass;
 	return pass ? PIGLIT_PASS : PIGLIT_FAIL;
 }