Re: [Mesa-dev] [PATCH 2/4] intel/decoder: Avoid freeing invalid pointer

2018-09-05 Thread Sagar Ghuge 

On 09/05/2018 11:06 AM, Lionel Landwerlin wrote:
> On 05/09/2018 19:02, Sagar Ghuge wrote:
>> Hi Lionel,
>>
>> Thanks for reviewing patches and comments.
>>
>> On 09/05/2018 10:29 AM, Lionel Landwerlin wrote:
>>> On 05/09/2018 18:19, Sagar Ghuge wrote:
 Signed-off-by: Sagar Ghuge 
 ---
    src/intel/common/gen_decoder.c | 4 
    1 file changed, 4 deletions(-)

 diff --git a/src/intel/common/gen_decoder.c 
 b/src/intel/common/gen_decoder.c
 index dbd060d53c..c44b8f060d 100644
 --- a/src/intel/common/gen_decoder.c
 +++ b/src/intel/common/gen_decoder.c
 @@ -662,8 +662,6 @@ 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);
 - ctx.spec = NULL;
     goto end;
  }
  if (XML_ParseBuffer(ctx.parser, len, len == 0) == 0) {
 @@ -672,8 +670,6 @@ gen_spec_load_from_path(const struct gen_device_info 
 *devinfo,
     XML_GetCurrentLineNumber(ctx.parser),
     XML_GetCurrentColumnNumber(ctx.parser),
     XML_ErrorString(XML_GetErrorCode(ctx.parser)));
 - free(ctx.spec);
 - ctx.spec = NULL;
     goto end;
  }
   } while (len > 0);
>>> Looks good but we're still missing a ralloc_free(ctx.spec) after the end 
>>> label.
>> In patch 4, I am freeing up gen_batch_decode_ctx instance using 
>> gen_batch_decode_ctx_finish() which is also responsible for destroying spec.
> 
> 
> I see. But I assumed that would be only in the case this function succeeded.
> 
> If we didn't manage to parse the genxml, is there much of a point trying to 
> decode anything?
> 
ohh. I got it. My bad, I was trying to fix wrong problem I guess. fread does 
not distinguish between end-of-file and error, so according to that I will free 
up spec. 
> 
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/4] intel/decoder: Avoid freeing invalid pointer

2018-09-05 Thread Lionel Landwerlin 

On 05/09/2018 19:02, Sagar Ghuge wrote:

Hi Lionel,

Thanks for reviewing patches and comments.

On 09/05/2018 10:29 AM, Lionel Landwerlin wrote:

On 05/09/2018 18:19, Sagar Ghuge wrote:

Signed-off-by: Sagar Ghuge 
---
   src/intel/common/gen_decoder.c | 4 
   1 file changed, 4 deletions(-)

diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c
index dbd060d53c..c44b8f060d 100644
--- a/src/intel/common/gen_decoder.c
+++ b/src/intel/common/gen_decoder.c
@@ -662,8 +662,6 @@ 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);
- ctx.spec = NULL;
    goto end;
     }
     if (XML_ParseBuffer(ctx.parser, len, len == 0) == 0) {
@@ -672,8 +670,6 @@ gen_spec_load_from_path(const struct gen_device_info 
*devinfo,
    XML_GetCurrentLineNumber(ctx.parser),
    XML_GetCurrentColumnNumber(ctx.parser),
    XML_ErrorString(XML_GetErrorCode(ctx.parser)));
- free(ctx.spec);
- ctx.spec = NULL;
    goto end;
     }
  } while (len > 0);

Looks good but we're still missing a ralloc_free(ctx.spec) after the end label.

In patch 4, I am freeing up gen_batch_decode_ctx instance using 
gen_batch_decode_ctx_finish() which is also responsible for destroying spec.



I see. But I assumed that would be only in the case this function succeeded.

If we didn't manage to parse the genxml, is there much of a point trying 
to decode anything?




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/4] intel/decoder: Avoid freeing invalid pointer

2018-09-05 Thread Sagar Ghuge 
Hi Lionel,

Thanks for reviewing patches and comments. 

On 09/05/2018 10:29 AM, Lionel Landwerlin wrote:
> On 05/09/2018 18:19, Sagar Ghuge wrote:
>> Signed-off-by: Sagar Ghuge 
>> ---
>>   src/intel/common/gen_decoder.c | 4 
>>   1 file changed, 4 deletions(-)
>>
>> diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c
>> index dbd060d53c..c44b8f060d 100644
>> --- a/src/intel/common/gen_decoder.c
>> +++ b/src/intel/common/gen_decoder.c
>> @@ -662,8 +662,6 @@ 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);
>> - ctx.spec = NULL;
>>    goto end;
>>     }
>>     if (XML_ParseBuffer(ctx.parser, len, len == 0) == 0) {
>> @@ -672,8 +670,6 @@ gen_spec_load_from_path(const struct gen_device_info 
>> *devinfo,
>>    XML_GetCurrentLineNumber(ctx.parser),
>>    XML_GetCurrentColumnNumber(ctx.parser),
>>    XML_ErrorString(XML_GetErrorCode(ctx.parser)));
>> - free(ctx.spec);
>> - ctx.spec = NULL;
>>    goto end;
>>     }
>>  } while (len > 0);
> 
> Looks good but we're still missing a ralloc_free(ctx.spec) after the end 
> label.

In patch 4, I am freeing up gen_batch_decode_ctx instance using 
gen_batch_decode_ctx_finish() which is also responsible for destroying spec. 
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/4] intel/decoder: Avoid freeing invalid pointer

2018-09-05 Thread Lionel Landwerlin 

On 05/09/2018 18:19, Sagar Ghuge wrote:

Signed-off-by: Sagar Ghuge 
---
  src/intel/common/gen_decoder.c | 4 
  1 file changed, 4 deletions(-)

diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c
index dbd060d53c..c44b8f060d 100644
--- a/src/intel/common/gen_decoder.c
+++ b/src/intel/common/gen_decoder.c
@@ -662,8 +662,6 @@ 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);
- ctx.spec = NULL;
   goto end;
}
if (XML_ParseBuffer(ctx.parser, len, len == 0) == 0) {
@@ -672,8 +670,6 @@ gen_spec_load_from_path(const struct gen_device_info 
*devinfo,
   XML_GetCurrentLineNumber(ctx.parser),
   XML_GetCurrentColumnNumber(ctx.parser),
   XML_ErrorString(XML_GetErrorCode(ctx.parser)));
- free(ctx.spec);
- ctx.spec = NULL;
   goto end;
}
 } while (len > 0);


Looks good but we're still missing a ralloc_free(ctx.spec) after the end 
label.


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 2/4] intel/decoder: Avoid freeing invalid pointer

2018-09-05 Thread Sagar Ghuge 
Signed-off-by: Sagar Ghuge 
---
 src/intel/common/gen_decoder.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c
index dbd060d53c..c44b8f060d 100644
--- a/src/intel/common/gen_decoder.c
+++ b/src/intel/common/gen_decoder.c
@@ -662,8 +662,6 @@ 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);
- ctx.spec = NULL;
  goto end;
   }
   if (XML_ParseBuffer(ctx.parser, len, len == 0) == 0) {
@@ -672,8 +670,6 @@ gen_spec_load_from_path(const struct gen_device_info 
*devinfo,
  XML_GetCurrentLineNumber(ctx.parser),
  XML_GetCurrentColumnNumber(ctx.parser),
  XML_ErrorString(XML_GetErrorCode(ctx.parser)));
- free(ctx.spec);
- ctx.spec = NULL;
  goto end;
   }
} while (len > 0);
-- 
2.17.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev