Timothy Arceri <[email protected]> writes:

> V4: Removed failure test that will never be true
>
> V3: changed indentation to tabs, use test label constants
> and remove unused variable
>
> V2: fixed whitespaces, broke tests into subtests,
> added missing build dir, and tidied up fail messages
> +static bool
> +test_object_label()
> +{
> +     GLsizei numBuffers = 1;
> +     GLsizei length;
> +     GLuint buffers[numBuffers];
> +     GLuint invalidBufferName;
> +     GLint maxLabelLength;
> +     GLchar label[11];
> +     GLchar *bigLabel;
> +     int i;
> +     bool pass = true;
> +
> +     glGenBuffers(numBuffers, buffers);

This 1-length array and dereferencing element 0 all over seems silly.
How about just:

glGenBuffers(1, &buffer);

> +     /* An INVALID_VALUE error is generated if the number of characters in
> +      * <label>, excluding the null terminator when <length> is negative, is 
> not
> +      * less than the value of MAX_LABEL_LENGTH.
> +      */
> +     glGetIntegerv(GL_MAX_LABEL_LENGTH, &maxLabelLength);
> +     bigLabel = (char *) malloc(maxLabelLength);

> +     for (i = 0; i <maxLabelLength; i++) {
> +             bigLabel[i] = 'a';
> +     }

This could optionally be replaced by memset.

> +     bigLabel[maxLabelLength] = '\0';

You overflowed bigLabel here.

We may also need to be defensive about the implementation's label
length.  I would expect that the maximum label length is about
2^(sizeof(size_t) * 8) - 1, since the implementation is probably just
using malloc to store them.  If so, then this test is going to lead to
oomkiller.

> +     if (!piglit_check_gl_error(GL_INVALID_VALUE)) {
> +             puts("  GL_INVALID_VALUE should be genereated when the 
> ObjectLabel name is invalid");
> +             pass = false;
> +     }

We usually use 'fprintf(stderr, ' for our error output from tests.  This
gets filtered separately and highlighted in the piglit-summary-html
output.

> +static bool
> +test_get_object_label()
> +{
> +     GLsizei numBuffers = 5;
> +     GLsizei length;
> +     GLuint buffers[numBuffers];
> +     GLuint invalidBufferName;
> +     GLchar label[12];

There are a bunch of magic numbers throughout this test that are related
to TestLabelLen, and should probably be expressed as things like
"label[TestLabelLen + 1]"

This test looks fairly complete.  Nice work!  The only other test I can
think of adding to this set is making sure that we can trivially get/set
labels on all the different kinds of objects (an easy thing to miss in
the implementation).

Attachment: pgp6rfiNEwWvm.pgp
Description: PGP signature

_______________________________________________
Piglit mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/piglit

Reply via email to