[PATCH vkd3d 1/6] vkd3d-shader: Return a vkd3d_string_buffer from hlsl_type_to_string().

[Matteo Bruni]

On Fri, Feb 26, 2021 at 7:13 PM Zebediah Figura (she/her)
<zfigura at codeweavers.com> wrote:
>
> On 2/26/21 12:08 PM, Matteo Bruni wrote:
> > On Tue, Feb 23, 2021 at 10:57 PM Zebediah Figura
> > <zfigura at codeweavers.com> wrote:
> >>
> >> Signed-off-by: Zebediah Figura <zfigura at codeweavers.com>
> >> ---
> >>  libs/vkd3d-shader/hlsl.c                 | 62 +++++++++++++-----------
> >>  libs/vkd3d-shader/hlsl.h                 |  4 +-
> >>  libs/vkd3d-shader/hlsl.y                 | 20 +++++---
> >>  libs/vkd3d-shader/vkd3d_shader_main.c    | 54 +++++++++++++++++++++
> >>  libs/vkd3d-shader/vkd3d_shader_private.h | 11 +++++
> >>  5 files changed, 114 insertions(+), 37 deletions(-)
> >>
> >> diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c
> >> index 52b3dd10..78eacf28 100644
> >> --- a/libs/vkd3d-shader/hlsl.c
> >> +++ b/libs/vkd3d-shader/hlsl.c
> >
> >>  const char *debug_hlsl_type(const struct hlsl_type *type)
> >>  {
> >> +    struct vkd3d_string_buffer_list string_buffers;
> >> +    struct vkd3d_string_buffer *string;
> >>      const char *ret;
> >> -    char *string;
> >>
> >> -    if (!(string = hlsl_type_to_string(type)))
> >> +    vkd3d_string_buffer_list_init(&string_buffers);
> >> +    if (!(string = hlsl_type_to_string(&string_buffers, type)))
> >>          return NULL;
> >> -    ret = vkd3d_dbg_sprintf("%s", string);
> >> -    vkd3d_free(string);
> >> +    ret = vkd3d_dbg_sprintf("%s", string->buffer);
> >> +    vkd3d_string_buffer_release(&string_buffers, string);
> >> +    vkd3d_string_buffer_list_cleanup(&string_buffers);
> >>      return ret;
> >>  }
> >
> > How painful would it be to pass the context (and thus string_buffers)
> > through to this function?
> >
>
> Not great—I do end up using it in more places—but not awful.

Okay, your call whether to do the change or not.

> >> diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h
> >> index e837dbcd..2ef3d5da 100644
> >> --- a/libs/vkd3d-shader/vkd3d_shader_private.h
> >> +++ b/libs/vkd3d-shader/vkd3d_shader_private.h
> >> @@ -865,17 +865,28 @@ bool shader_sm4_is_end(void *data, const DWORD **ptr) DECLSPEC_HIDDEN;
> >>
> >>  struct vkd3d_string_buffer
> >>  {
> >> +    struct list entry;
> >>      char *buffer;
> >>      unsigned int buffer_size;
> >>      unsigned int content_size;
> >>  };
> >>
> >> +struct vkd3d_string_buffer_list
> >> +{
> >> +    struct list list;
> >> +};
> >
> > Since this is the second time we implement it and hindsight is usually
> > very good, I'd prefer it if we used an array + vkd3d_array_reserve()
> > to store the stack of available vkd3d_string_buffer structs.
>
> Sure, will do.
>
> > Also it would be nice to rename struct vkd3d_string_buffer_list to
> > something that explains its function rather than what it is, but as
> > usual I don't have any good suggestions...
> >
>
> I'm not sure what's a more functionally descriptive name, other than
> maybe vkd3d_string_buffer_cache?

Nice, works for me.