[Piglit] [PATCH] shader_runner: Check sscanf return value.

[Dylan Baker]

Quoting Vinson Lee (2016-11-07 14:34:23)
> On Mon, Nov 7, 2016 at 2:29 PM, Dylan Baker <dylan at pnwbakers.com> wrote:
> > Quoting Vinson Lee (2016-11-07 14:03:41)
> >> On Mon, Nov 7, 2016 at 10:03 AM, Dylan Baker <dylan at pnwbakers.com> wrote:
> >> > Quoting Vinson Lee (2016-11-04 16:30:39)
> >> >> Fix Coverity unchecked return value defect.
> >> >>
> >> >> CID: 1373655
> >> >> Signed-off-by: Vinson Lee <vlee at freedesktop.org>
> >> >> ---
> >> >>  tests/shaders/shader_runner.c | 7 ++++++-
> >> >>  1 file changed, 6 insertions(+), 1 deletion(-)
> >> >>
> >> >> diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
> >> >> index 02a6cd7cda82..62fc8120a30c 100644
> >> >> --- a/tests/shaders/shader_runner.c
> >> >> +++ b/tests/shaders/shader_runner.c
> >> >> @@ -2769,8 +2769,13 @@ handle_texparameter(const char *line)
> >> >>                 return;
> >> >>         } else if (string_match("border_color ", line)) {
> >> >>                 float bc[4];
> >> >> +               int count;
> >> >>                 line += strlen("border_color ");
> >> >> -               sscanf(line, "%f %f %f %f", &bc[0], &bc[1], &bc[2], &bc[3]);
> >> >> +               count = sscanf(line, "%f %f %f %f", &bc[0], &bc[1], &bc[2], &bc[3]);
> >> >> +               if (count != 4) {
> >> >> +                       fprintf(stderr, "Could not parse border_color texture parameter.\n");
> >> >> +                       piglit_report_result(PIGLIT_FAIL);
> >> >
> >> > This breaks running more than one shader test at a time.
> >> > This should be "result = PIGLIT_FAIL;"
> >> >
> >>
> >> There is no result to be returned by handle_texparameter though and
> >> the final else statement also reports PIGLIT_FAIL.
> >>
> >> >> +               }
> >> >>                 glTexParameterfv(target, GL_TEXTURE_BORDER_COLOR, bc);
> >> >>                 return;
> >> >>         } else if (string_match("swizzle_r ", line)) {
> >> >> --
> >> >> 2.7.4
> >> >>
> >> >> _______________________________________________
> >> >> Piglit mailing list
> >> >> Piglit at lists.freedesktop.org
> >> >> https://lists.freedesktop.org/mailman/listinfo/piglit
> >
> > Right, but currently the function doesn't piglit_report_result except with
> > PIGLIT_SKIP, which has an explicit workaround because it can get raised in low
> > levels of the framework. This will break running more than one shader test at a
> > time if it hits the piglit_report_result you're adding.
> 
> PIGLIT_FAIL is reported several lines down in shader_runner.c:2783.

Ah, that's another bug. Both Marek and I must have missed that. Grepping for
piglit_report_result there are a few other places it's being used in
shader_runner.c it shouldn't. A lot of them actually.

I'll retract my comments. I think there's a bigger problem than an error
checking case.

Dylan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 455 bytes
Desc: signature
URL: <https://lists.freedesktop.org/archives/piglit/attachments/20161107/3970b7c2/attachment.sig>