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.
@@ -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.
Giovanni.