[Piglit] [PATCH v2 1/4] shader_runner: Use strndup to get a null-terminated string for 'line'

[Jordan Justen]

On 2015-06-16 04:07:35, Francisco Jerez wrote:
> Jordan Justen <jordan.l.justen at intel.com> writes:
> 
> > In order to use sscanf with optional parameters, the string needs to
> > be null terminated. Otherwise, sscanf will treat the newline as
> > whitespace and continue to look for matches on the following lines.
> >
> > v2:
> >  * Call strndup before skipping newline char in next_line
> >
> > Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> > ---
> >  tests/shaders/shader_runner.c | 25 +++++++++++++++++--------
> >  1 file changed, 17 insertions(+), 8 deletions(-)
> >
> > diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
> > index 1827e08..41024cd 100644
> > --- a/tests/shaders/shader_runner.c
> > +++ b/tests/shaders/shader_runner.c
> > @@ -2294,7 +2294,7 @@ probe_atomic_counter(GLint counter_num, const char *op, uint32_t value)
> >  enum piglit_result
> >  piglit_display(void)
> >  {
> > -     const char *line;
> > +     const char *line, *next_line;
> >       bool pass = true;
> >       GLbitfield clear_bits = 0;
> >       bool link_error_expected = false;
> > @@ -2303,16 +2303,27 @@ piglit_display(void)
> >       if (test_start == NULL)
> >               return PIGLIT_PASS;
> >  
> > -     line = test_start;
> > -     while (line[0] != '\0') {
> > +     next_line = test_start;
> > +     while (next_line[0] != '\0') {
> >               float c[32];
> >               double d[4];
> >               int x, y, z, w, h, l, tex, level;
> >               char s[32];
> >  
> > -             line = eat_whitespace(line);
> >  
> > -             if (sscanf(line, "atomic counters %d", &x) == 1) {
> > +             line = eat_whitespace(next_line);
> > +
> > +             next_line = strchrnul(next_line, '\n');
> > +
> > +             /* Duplicate the line to make it null terminated */
> > +             line = strndup(line, next_line - line);
> > +
> > +             /* If strchrnul found a newline, then skip it */
> > +             if (next_line[0] != '\0')
> > +                     next_line++;
> > +
> > +             if (line[0] == '\0') {
> 
> Hmm.  Isn't this already handled in the last block of the else-if chain?

True, it should skip all portions of the if-else block (including the
last), except sscanf was being incorrectly used by two commands,
meaning another command was being hit. I'll reply to this email with a
new patch to fix that issue, but I still think we should add the null
string check up top to be safer.

-Jordan

> > +             } else if (sscanf(line, "atomic counters %d", &x) == 1) {
> >                       GLuint *atomics_buf = calloc(x, sizeof(GLuint));
> >                       glGenBuffers(1, &atomics_bo);
> >                       glBindBufferBase(GL_ATOMIC_COUNTER_BUFFER, 0, atomics_bo);
> > @@ -2701,9 +2712,7 @@ piglit_display(void)
> >                       piglit_report_result(PIGLIT_FAIL);
> >               }
> >  
> > -             line = strchrnul(line, '\n');
> > -             if (line[0] != '\0')
> > -                     line++;
> > +             free((void*) line);
> >       }
> >  
> >       if (!link_ok && !link_error_expected) {
> > -- 
> > 2.1.4
> >
> > _______________________________________________
> > Piglit mailing list
> > Piglit at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/piglit