[PATCH vkd3d] vkd3d-shader: Allow a NULL buffer in vkd3d_string_buffer.

[Zebediah Figura (she/her)]

On 10/6/21 07:48, Giovanni Mascellani wrote:
> Hi,
> 
> Il 05/10/21 18:21, Zebediah Figura (she/her) ha scritto:
>> Well, in the sense that 216057 (well, 216055 really) is mainly just 
>> there to get rid of the boilerplate around calling 
>> hlsl_type_to_string(). This patch does lesson that boilerplate a bit.
> 
> Mmh, I am not sure I understand why that would lessen the boilerplate. 
> You still need to name and destroy the string buffer, each of which 
> takes a line. And vkd3d_string_buffer_data(string) is definitely longer 
> than string->buffer. I don't really think this helps here.

Well, it lessens the boilerplate if you no longer need to check for 
success from hlsl_type_to_string().

> 
>>>>> @@ -855,7 +856,7 @@ struct vkd3d_string_buffer 
>>>>> *hlsl_type_to_string(struct hlsl_ctx *ctx, const stru
>>>>>               if ((inner_string = hlsl_type_to_string(ctx, t)))
>>>>>               {
>>>>> -                vkd3d_string_buffer_printf(string, "%s", 
>>>>> inner_string->buffer);
>>>>> +                vkd3d_string_buffer_printf(string, "%s", 
>>>>> vkd3d_string_buffer_data(inner_string));
>>>>>                   hlsl_release_string_buffer(ctx, inner_string);
>>>>>               }
>>>>
>>>> This means that here, and everywhere else, you can get rid of the 
>>>> NULL check.
>>>
>>> I'm pretty sure that in many cases vkd3d_string_buffer_data could be 
>>> avoided, when it can be shown that the buffer cannot be NULL. Though 
>>> ideally I'd like to encapsulate as much as possible the fact that the 
>>> buffer can be NULL inside the accessor vkd3d_string_buffer_data, so I 
>>> am using the accessor nearly everywhere (a couple of places were 
>>> actually using knowledge of the internals of vkd3d_string_buffer, and 
>>> I didn't change that, because after all we're writing C and nobody 
>>> really expects full encapsulation).
>>
>> I guess you're looking at this with the perspective that it's just 
>> there to avoid asserting the result of an allocation. That's fine, but 
>> I think that it's useful to avoid checking the result of one as well. 
>> Sort of like put_u32() and friends.
> 
> Sorry, I don't think I'm following you.

I mean, it's very convenient to be able to not have to check for success 
from an allocation, in some way or another, if you're going to be doing 
that frequently. vkd3d_string_buffer_data() achieves that, so I think 
it's useful to see that as an end goal of vkd3d_string_buffer_data() in 
itself, and to take advantage of it to get rid of ubiquitous error checking.