Re: [Mesa-dev] [PATCH] glsl: Fix memory leak in glsl_lexer.ll
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
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
> > > --- 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
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
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