Re: [Mesa-dev] [PATCH] glsl: Fix memory leak in glsl_lexer.ll

2014-09-09 Thread Ian Romanick
On 09/05/2014 06:22 AM, Juha-Pekka Heikkila wrote:
> Running fast clear glClear with SNB caused Valgrind to
> complain about this.
> 
> v2: line 237 fixed glClear from leaking memory, other
> strdups are also now changed to ralloc_strdups but I
> don't know what effect those have. At least no changes in
> my Piglit quick run.
> 
> Signed-off-by: Juha-Pekka Heikkila 
> ---
>  src/glsl/glsl_lexer.ll | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/src/glsl/glsl_lexer.ll b/src/glsl/glsl_lexer.ll
> index b7c4aad..9cf57dd 100644
> --- a/src/glsl/glsl_lexer.ll
> +++ b/src/glsl/glsl_lexer.ll
> @@ -81,7 +81,8 @@ static int classify_identifier(struct 
> _mesa_glsl_parse_state *, const char *);
> "illegal use of reserved word `%s'", yytext); \
>return ERROR_TOK;  \
>} else {   
> \
> -  yylval->identifier = strdup(yytext);   \
> +  void *ctx = yyextra;   
> \
> +  yylval->identifier = ralloc_strdup(ctx, yytext);   \

It seems a little weird to do this as two lines.  Why not

 yylval->identifier = ralloc_strdup(yyextra, yytext);

Also, we're trying to use mem_ctx for the ralloc memory context.  Using
just ctx was confusing in many places because the GL context is also
usually named ctx.

Either s/ctx/mem_ctx/ or do it in one line to get a

Reviewed-by: Ian Romanick 

>return classify_identifier(yyextra, yytext);   \
>}  
> \
> } while (0)
> @@ -232,7 +233,8 @@ HASH  ^{SPC}#{SPC}
>  [ \t\r]* { }
>  :return COLON;
>  [_a-zA-Z][_a-zA-Z0-9]*   {
> -yylval->identifier = strdup(yytext);
> +void *ctx = yyextra;
> +yylval->identifier = ralloc_strdup(ctx, 
> yytext);
>  return IDENTIFIER;
>   }
>  [1-9][0-9]*  {
> @@ -409,7 +411,8 @@ layout{
>|| yyextra->ARB_compute_shader_enable) {
> return LAYOUT_TOK;
>  } else {
> -   yylval->identifier = strdup(yytext);
> +   void *ctx = yyextra;
> +   yylval->identifier = ralloc_strdup(ctx, yytext);
> return classify_identifier(yyextra, yytext);
>  }
>   }
> 

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


[Mesa-dev] [PATCH] glsl: Fix memory leak in glsl_lexer.ll

2014-09-05 Thread Juha-Pekka Heikkila
Running fast clear glClear with SNB caused Valgrind to
complain about this.

v2: line 237 fixed glClear from leaking memory, other
strdups are also now changed to ralloc_strdups but I
don't know what effect those have. At least no changes in
my Piglit quick run.

Signed-off-by: Juha-Pekka Heikkila 
---
 src/glsl/glsl_lexer.ll | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/glsl/glsl_lexer.ll b/src/glsl/glsl_lexer.ll
index b7c4aad..9cf57dd 100644
--- a/src/glsl/glsl_lexer.ll
+++ b/src/glsl/glsl_lexer.ll
@@ -81,7 +81,8 @@ static int classify_identifier(struct _mesa_glsl_parse_state 
*, const char *);
  "illegal use of reserved word `%s'", yytext); \
 return ERROR_TOK;  \
   } else { \
-yylval->identifier = strdup(yytext);   \
+void *ctx = yyextra;   
\
+yylval->identifier = ralloc_strdup(ctx, yytext);   \
 return classify_identifier(yyextra, yytext);   \
   }
\
} while (0)
@@ -232,7 +233,8 @@ HASH^{SPC}#{SPC}
 [ \t\r]*   { }
 :  return COLON;
 [_a-zA-Z][_a-zA-Z0-9]* {
-  yylval->identifier = strdup(yytext);
+  void *ctx = yyextra;
+  yylval->identifier = ralloc_strdup(ctx, 
yytext);
   return IDENTIFIER;
}
 [1-9][0-9]*{
@@ -409,7 +411,8 @@ layout  {
   || yyextra->ARB_compute_shader_enable) {
  return LAYOUT_TOK;
   } else {
- yylval->identifier = strdup(yytext);
+ void *ctx = yyextra;
+ yylval->identifier = ralloc_strdup(ctx, yytext);
  return classify_identifier(yyextra, yytext);
   }
}
-- 
1.8.5.1

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


Re: [Mesa-dev] [PATCH] glsl: Fix memory leak in glsl_lexer.ll

2014-09-03 Thread Aras Pranckevicius
>
> > --- a/src/glsl/glsl_lexer.ll
> > +++ b/src/glsl/glsl_lexer.ll
> > @@ -232,7 +232,8 @@ HASH^{SPC}#{SPC}
> >  [ \t\r]*   { }
> >  :  return COLON;
> >  [_a-zA-Z][_a-zA-Z0-9]* {
> > -  yylval->identifier = strdup(yytext);
> > +  void *ctx = yyextra;
> > +  yylval->identifier =
> ralloc_strdup(ctx, yytext);
> >return IDENTIFIER;
> > }
> >  [1-9][0-9]*{
>
> There's another use of strdup() in this file. I assume it needs the
> same treatment.
>

Seems like there's a memory leak at least in extension indentifiers:
http://lists.freedesktop.org/archives/mesa-dev/2013-August/044010.html


-- 
Aras Pranckevičius
work: http://unity3d.com
home: http://aras-p.info
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] glsl: Fix memory leak in glsl_lexer.ll

2014-09-03 Thread Matt Turner
On Wed, Sep 3, 2014 at 7:03 AM, Juha-Pekka Heikkila
 wrote:
> Running fast clear glClear with SNB caused Valgrind to
> complain about this.
>
> Signed-off-by: Juha-Pekka Heikkila 
> ---
>  src/glsl/glsl_lexer.ll | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/glsl/glsl_lexer.ll b/src/glsl/glsl_lexer.ll
> index b7c4aad..a9274a3 100644
> --- a/src/glsl/glsl_lexer.ll
> +++ b/src/glsl/glsl_lexer.ll
> @@ -232,7 +232,8 @@ HASH^{SPC}#{SPC}
>  [ \t\r]*   { }
>  :  return COLON;
>  [_a-zA-Z][_a-zA-Z0-9]* {
> -  yylval->identifier = strdup(yytext);
> +  void *ctx = yyextra;
> +  yylval->identifier = ralloc_strdup(ctx, 
> yytext);
>return IDENTIFIER;
> }
>  [1-9][0-9]*{

There's another use of strdup() in this file. I assume it needs the
same treatment.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] glsl: Fix memory leak in glsl_lexer.ll

2014-09-03 Thread Juha-Pekka Heikkila
Running fast clear glClear with SNB caused Valgrind to
complain about this.

Signed-off-by: Juha-Pekka Heikkila 
---
 src/glsl/glsl_lexer.ll | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/glsl/glsl_lexer.ll b/src/glsl/glsl_lexer.ll
index b7c4aad..a9274a3 100644
--- a/src/glsl/glsl_lexer.ll
+++ b/src/glsl/glsl_lexer.ll
@@ -232,7 +232,8 @@ HASH^{SPC}#{SPC}
 [ \t\r]*   { }
 :  return COLON;
 [_a-zA-Z][_a-zA-Z0-9]* {
-  yylval->identifier = strdup(yytext);
+  void *ctx = yyextra;
+  yylval->identifier = ralloc_strdup(ctx, 
yytext);
   return IDENTIFIER;
}
 [1-9][0-9]*{
-- 
1.8.5.1

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