Re: [Piglit] [PATCH 3/3] arb_texture_view: fix security format warnings

[Burton, Ross]

On Fri, 30 Nov 2018 at 12:45, Eric Engestrom  wrote:
> On Friday, 2018-11-30 10:45:06 +, Ross Burton wrote:
> I'm not sure this macro has any reason to exist though, it's only used once,
> doesn't do any macro magic, uses each of its params exactly once...
> I would really recommend dropping it.

Yes, that's a good point: V2 sent.

Ross
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

Re: [Piglit] [PATCH 3/3] arb_texture_view: fix security format warnings

[Eric Engestrom]

On Friday, 2018-11-30 10:45:06 +, 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.
> 
> Signed-off-by: Ross Burton 

This patch is:
Reviewed-by: Eric Engestrom 

I'm not sure this macro has any reason to exist though, it's only used once,
doesn't do any macro magic, uses each of its params exactly once...
I would really recommend dropping it.

> ---
>  tests/spec/arb_texture_view/rendering-layers-image.c | 8 +++-
>  1 file changed, 3 insertions(+), 5 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..070e29a68 100644
> --- a/tests/spec/arb_texture_view/rendering-layers-image.c
> +++ b/tests/spec/arb_texture_view/rendering-layers-image.c
> @@ -142,12 +142,12 @@ test_render_layers(const struct test_info *test)
>   return pass;
>  }
>  
> -#define X(f, desc) \
> +#define X(f, test_type) \
>   do { \
>   const bool subtest_pass = (f); \
>   piglit_report_subtest_result(subtest_pass \
>? PIGLIT_PASS : PIGLIT_FAIL, \
> -  (desc)); \
> +  "layers rendering of %s", 
> (test_type)); \
>   pass = pass && subtest_pass; \
>   } while (0)
>  
> @@ -157,9 +157,7 @@ piglit_display(void)
>   bool pass = true;
>   for (int test_idx = 0; test_idx < ARRAY_SIZE(tests); test_idx++) {
>   const struct test_info *test = [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);
> + X(test_render_layers(test), test->uniform_type);
>   }
>  #undef X
>   pass = piglit_check_gl_error(GL_NO_ERROR) && pass;
> -- 
> 2.11.0
> 
> ___
> Piglit mailing list
> Piglit@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH 3/3] arb_texture_view: fix security format warnings

[Ross Burton]

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.

Signed-off-by: Ross Burton 
---
 tests/spec/arb_texture_view/rendering-layers-image.c | 8 +++-
 1 file changed, 3 insertions(+), 5 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..070e29a68 100644
--- a/tests/spec/arb_texture_view/rendering-layers-image.c
+++ b/tests/spec/arb_texture_view/rendering-layers-image.c
@@ -142,12 +142,12 @@ test_render_layers(const struct test_info *test)
return pass;
 }
 
-#define X(f, desc) \
+#define X(f, test_type) \
do { \
const bool subtest_pass = (f); \
piglit_report_subtest_result(subtest_pass \
 ? PIGLIT_PASS : PIGLIT_FAIL, \
-(desc)); \
+"layers rendering of %s", 
(test_type)); \
pass = pass && subtest_pass; \
} while (0)
 
@@ -157,9 +157,7 @@ piglit_display(void)
bool pass = true;
for (int test_idx = 0; test_idx < ARRAY_SIZE(tests); test_idx++) {
const struct test_info *test = [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);
+   X(test_render_layers(test), test->uniform_type);
}
 #undef X
pass = piglit_check_gl_error(GL_NO_ERROR) && pass;
-- 
2.11.0

___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit