Re: [PATCH vkd3d 01/12] vkd3d-shader/hlsl: Store the struct fields as an array.

[Giovanni Mascellani]

Hi,

Il 01/07/22 23:24, Francisco Casas ha scritto:
> @@ -512,37 +511,30 @@ struct hlsl_type *hlsl_type_clone(struct hlsl_ctx *ctx, struct hlsl_type *old,
>       case HLSL_CLASS_STRUCT:
>       {
> -
>        if (!(type->e.elements = hlsl_alloc(ctx, sizeof(*type->e.elements))))
> -
>        size_t field_count = old->e.record.field_count, i;
> -
> -
>        type->e.record.field_count = field_count;
> -
> -
>        if (!(type->e.record.fields = hlsl_alloc(ctx, field_count * sizeof(*type->e.record.fields))))
>         {
>             vkd3d_free((void *)type->name);
>             vkd3d_free(type);
>             return NULL;
>         }
> -
>        list_init(type->e.elements);
> -
>        LIST_FOR_EACH_ENTRY(old_field, old->e.elements, struct hlsl_struct_field, entry)
> -
> -
>        for (i = 0; i < field_count; ++i)
>         {
> -
>            if (!(field = hlsl_alloc(ctx, sizeof(*field))))
> -
>            {
> -
>                LIST_FOR_EACH_ENTRY_SAFE(field, old_field, type->e.elements, struct hlsl_struct_field, entry)
> -
>                {
> -
>                    vkd3d_free((void *)field->semantic.name);
> -
>                    vkd3d_free((void *)field->name);
> -
>                    vkd3d_free(field);
> -
>                }
> -
>                vkd3d_free(type->e.elements);
> -
>                vkd3d_free((void *)type->name);
> -
>                vkd3d_free(type);
> -
>                return NULL;
> -
>            }
> -
>            field->loc = old_field->loc;
> -
>            field->type = hlsl_type_clone(ctx, old_field->type, default_majority, modifiers);
> -
>            field->name = hlsl_strdup(ctx, old_field->name);
> -
>            if (old_field->semantic.name)
> -
>            const struct hlsl_struct_field *src_field = &old->e.record.fields[i];
> -
>            struct hlsl_struct_field *dst_field = &type->e.record.fields[i];
> -
> -
>            dst_field->loc = src_field->loc;
> -
>            dst_field->type = hlsl_type_clone(ctx, src_field->type, default_majority, modifiers);

This is happening also before the patch and in the array branch, but 
shouldn't we detect if hlsl_type_clone() returns NULL and handle the 
error? It's true that the memory error is set anyway, so the compilation 
will eventually fail, but in the meantime dst_file->type is NULL, and 
some other code ran during parsing could trip on that.

Giovanni.