Re: [PATCH vkd3d v3 2/5] vkd3d-shader: Include register ranges in descriptor binding error messages.

[Henri Verbeet]

On Fri, 23 Jul 2021 at 15:03, Conor McCarthy  wrote:
> +static struct vkd3d_string_buffer *vkd3d_dxbc_compiler_binding_error(struct vkd3d_dxbc_compiler *compiler,
> -
>    enum vkd3d_shader_descriptor_type descriptor_type, const struct vkd3d_shader_register_range *range)
>
> +{
> - struct vkd3d_string_buffer *buffer = vkd3d_string_buffer_get(&compiler->string_buffers);
> - struct vkd3d_string_buffer *range_buffer = vkd3d_string_buffer_get(&compiler->string_buffers);
> -
> - if (!buffer || !range_buffer)
> - {
> -
>    vkd3d_string_buffer_release(&compiler->string_buffers, buffer);
> -
>    vkd3d_string_buffer_release(&compiler->string_buffers, range_buffer);
> -
>    return NULL;
> - }
> -
> - if (range->last != ~0u)
> -
>    vkd3d_string_buffer_printf(range_buffer, "[%u:%u]", range->first, range->last);
> - else
> -
>    vkd3d_string_buffer_printf(range_buffer, "[%u:*]", range->first);
> -
> - vkd3d_string_buffer_printf(buffer, "Could not find descriptor binding for type %#x, space %u, registers %s, "
> -
>        "shader type %#x.\n", descriptor_type, range->space, range_buffer->buffer, compiler->shader_type);
> -
> - vkd3d_string_buffer_release(&compiler->string_buffers, range_buffer);
> - return buffer;
>
> +}
> -

Compiler error messages don't need explicit newlines; adding the \n
will introduce an extra empty line.
Note that if we're going to have a helper for generating the error
message, we may as well call vkd3d_dxbc_compiler_error() here as well.
> @@ -2552,11 +2581,12 @@ static struct vkd3d_shader_descriptor_binding vkd3d_dxbc_compiler_get_descriptor
>          }
>          if (shader_interface->binding_count)
>          {
> -
>        FIXME("Could not find binding for type %#x, space %u, register %u, shader type %#x.\n",
> -
>                descriptor_type, range->space, range->first, compiler->shader_type);
> -
>        struct vkd3d_string_buffer *buffer = vkd3d_dxbc_compiler_binding_error(compiler, descriptor_type, range);
> -
>        if (buffer)
> -
>            FIXME(buffer->buffer);
>        vkd3d_dxbc_compiler_error(compiler, VKD3D_SHADER_ERROR_SPV_DESCRIPTOR_BINDING_NOT_FOUND,
> -
>                "Could not find descriptor binding for type %#x, space %u, register %u, shader type %#x.",
> -
>                descriptor_type, range->space, range->first, compiler->shader_type);
> -
>                buffer ? buffer->buffer : "");
> -
>        vkd3d_string_buffer_release(&compiler->string_buffers, buffer);
>    }
> }

It should work out here in practice, but note that it's generally
unsafe to pass arbitrary strings as format strings to printf() style
functions. (E.g., consider what would happen when the contents of
buffer->buffer would be "%s".)