On 09/06/2018 02:39 AM, Lionel Landwerlin wrote:
> On 06/09/2018 05:12, Sagar Ghuge wrote:
>> v2: Free ctx.spec if error while reading genxml (Lionel Landwerlin)
>>
>> Signed-off-by: Sagar Ghuge <sagar.gh...@intel.com>
>> ---
>>   src/intel/common/gen_decoder.c | 15 ++++++++++++---
>>   1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c
>> index d4db8b89cc..9d647033cf 100644
>> --- a/src/intel/common/gen_decoder.c
>> +++ b/src/intel/common/gen_decoder.c
>> @@ -654,7 +654,7 @@ gen_spec_load_from_path(const struct gen_device_info 
>> *devinfo,
>>      ctx.spec = gen_spec_init();
>>      if (ctx.spec == NULL) {
>>         fprintf(stderr, "Failed to create gen_spec\n");
>> -      return NULL;
>> +      goto end;
>>      }
>>        do {
>> @@ -662,17 +662,26 @@ gen_spec_load_from_path(const struct gen_device_info 
>> *devinfo,
>>         len = fread(buf, 1, XML_BUFFER_SIZE, input);
>>         if (len == 0) {
>>            fprintf(stderr, "fread: %m\n");
>> -         free(ctx.spec);
>> +         gen_spec_destroy(ctx.spec);
>>            ctx.spec = NULL;
>>            goto end;
>> +      } else {
>> +         if (ferror(input)) {
>> +            fprintf(stderr, "fread: %m\n");
>> +            gen_spec_destroy(ctx.spec);
>> +            ctx.spec = NULL;
>> +            goto end;
>> +         } else if (feof(input))
>> +            goto end;
> 
> 
> Looking at the fread man page, it seems like len == 0 means either error or 
> end of file.
> 
I was trying to handle the case
 - if genxml is already empty/has error on first fread attempt, then free 
ctx.spec. (len == 0)

if it's not empty and there is a error for subsequent fread's
 - free ctx.spec

for rest of the behavior just goto end.


> So I think the ferror/feof checks should be inside the if (len == 0).
> 
> 
> Maybe we can even remove the if (len == 0) and put the if (ferror(input)) ... 
> instead.
> 

if we remove (len == 0) and kept only ferror(input) & feof(input) check will 
end up 
allocating memory for ctx.spec if genxml file is empty. 

Please let me know if I might missed any other cases. 
> 
> -
> 
> Lionel
> 
> 
>>         }
>> +
>>         if (XML_ParseBuffer(ctx.parser, len, len == 0) == 0) {
>>            fprintf(stderr,
>>                    "Error parsing XML at line %ld col %ld: %s\n",
>>                    XML_GetCurrentLineNumber(ctx.parser),
>>                    XML_GetCurrentColumnNumber(ctx.parser),
>>                    XML_ErrorString(XML_GetErrorCode(ctx.parser)));
>> -         free(ctx.spec);
>> +         gen_spec_destroy(ctx.spec);
>>            ctx.spec = NULL;
>>            goto end;
>>         }
> 
> 
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to