>> +/**
>> + * Helper for _mesa_ObjectLabel() and _mesa_ObjectPtrLabel().
>> + */
>> +static void
>> +set_label(struct gl_context *ctx, char **labelPtr, const char *label,
>> + int length, const char *caller)
>> +{
>> + if (*labelPtr) {
>> + /* free old label string */
>> + free(*labelPtr);
>> + }
>> +
>> + if (label) {
>> + /* set new label string */
>> +
>> + if (length >= 0) {
>
>This should be > 0. malloc(0) is not portable.
>
>Shouldn't there also be a MAX_LABEL_LENGTH test for this patch?
I think you are reviewing an old version of the patch. New version has that
test.
>> + /* explicit length */
>> + *labelPtr = (char *) malloc(length);
>> + if (*labelPtr) {
>> + memcpy(*labelPtr, label, length);
>> + }
>> + }
>> + else {
>> + /* null-terminated string */
>> + int len = strlen(label);
>> + if (len >= MAX_LABEL_LENGTH) {
>The reason MAX_LABEL_LENGTH exists is so that you can have a fixed-size
>array in your structure (so you don't have to malloc a buffer. Either
>make a fixed size buffer, or make MAX_LABEL_LENGTH be the maximum size
>representable in a GLsizei (and eliminate this check).
Ok makes sense. However the check is still valid its in the spec. We shouldn't
just truncate the string I know for sure that the AMD driver does the same
thing. I have posted extensive tests for the objectlabel code on the piglit
list.
>> + /* 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.
>> + */
>> + _mesa_error(ctx, GL_INVALID_VALUE,
>> + "%s(length=%d, which is not less than "
>> + "GL_MAX_LABEL_LENGTH=%d)", caller, length,
>> + MAX_LABEL_LENGTH);
>> + return;
>> + }
>> + *labelPtr = _mesa_strdup(label);
>> + }
>> + }
>> +}
>> +
>> +/**
>> + * Helper for _mesa_GetObjectLabel() and _mesa_GetObjectPtrLabel().
>> + */
>> +static void
>> +copy_label(char **labelPtr, char *label, int *length, int bufSize)
>> +{
>> + int labelLen = 0;
>> +
>> + if (*labelPtr)
>> + labelLen = strlen(*labelPtr);
>> +
>> + if (label) {
>
>There should be a spec quote here explaining why this value is returned.
>Other places in OpenGL include the NUL terminator in the length.
>
> /* The KHR_debug spec says:
> *
> * "The string representation of the message is stored in
> * <message> and its length (excluding the null-terminator)
> * is stored in <length>"
> */
ok
So are you going to revert the patchset or do I create a fixup patchset?
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
