[Piglit] [PATCH V4] KHR_debug: test ObjectLabel(), GetObjectLabel(), ObjectPtrLabel() and GetObjectPtrLabel()

[Eric Anholt]

Timothy Arceri <t_arceri at yahoo.com.au> 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).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20130828/f7ef668b/attachment.pgp>