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

[Zebediah Figura]

On 7/5/22 04:22, Giovanni Mascellani wrote:
> 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.

Yes, there are still a few places in the HLSL parser that don't check 
for allocation failure and should.