On 09/20/2013 09:51 PM, Francisco Jerez wrote:
> The _mesa_glsl_parse_state object relies on the memory allocator
> zeroing out its contents before it's initialized, which is quite an
> unusual practice in the C++ world because it ties objects to some
> specific allocation scheme, and gives unpredictable results when an
> object is created with a different allocator -- Stack allocation,
> array allocation, or aggregation inside a different object are some of
> the useful possibilities that come to my mind. Initialize all fields
> from the constructor and stop using the zeroing allocator.
>
> Reviewed-by: Paul Berry <stereotype...@gmail.com>
> Reviewed-by: Chad Versace <chad.vers...@linux.intel.com>
> ---
> src/glsl/glsl_parser_extras.cpp | 16 ++++++++++++++--
> src/glsl/glsl_parser_extras.h | 2 +-
> 2 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
> index cac5a18..772933f 100644
> --- a/src/glsl/glsl_parser_extras.cpp
> +++ b/src/glsl/glsl_parser_extras.cpp
> @@ -55,7 +55,7 @@ static unsigned known_desktop_glsl_versions[] =
>
> _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct gl_context *_ctx,
> GLenum target, void *mem_ctx)
> - : ctx(_ctx)
> + : ctx(_ctx), switch_state()
Related to my comment on Vinson's patch (see "glsl: Initialize
assignment_generator member variables."), we need to come up with some
coding conventions.
- What things should use the initialization list, and what things
should be ininitailized in the body of the constructor?
- Should each initializer go on its own line, or should they be put on
fewer lines?
- What should be the indentation before the ":"?
> {
> switch (target) {
> case GL_VERTEX_SHADER: this->target = vertex_shader; break;
> @@ -66,10 +66,14 @@ _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct
> gl_context *_ctx,
> this->scanner = NULL;
> this->translation_unit.make_empty();
> this->symbols = new(mem_ctx) glsl_symbol_table;
> +
> + this->num_uniform_blocks = 0;
> + this->uniform_block_array_size = 0;
> + this->uniform_blocks = NULL;
> +
> this->info_log = ralloc_strdup(mem_ctx, "");
> this->error = false;
> this->loop_nesting_ast = NULL;
> - this->switch_state.switch_nesting_ast = NULL;
>
> this->struct_specifier_depth = 0;
> this->num_builtins_to_link = 0;
> @@ -105,6 +109,13 @@ _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct
> gl_context *_ctx,
>
> this->Const.MaxDrawBuffers = ctx->Const.MaxDrawBuffers;
>
> + this->current_function = NULL;
> + this->toplevel_ir = NULL;
> + this->found_return = false;
> + this->all_invariant = false;
> + this->user_structures = NULL;
> + this->num_user_structures = 0;
> +
> /* Populate the list of supported GLSL versions */
> /* FINISHME: Once the OpenGL 3.0 'forward compatible' context or
> * the OpenGL 3.2 Core context is supported, this logic will need
> @@ -163,6 +174,7 @@ _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct
> gl_context *_ctx,
>
> this->gs_input_prim_type_specified = false;
> this->gs_input_prim_type = GL_POINTS;
> + this->gs_input_size = 0;
> this->out_qualifier = new(this) ast_type_qualifier();
> }
>
> diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
> index 364a983..853a9b0 100644
> --- a/src/glsl/glsl_parser_extras.h
> +++ b/src/glsl/glsl_parser_extras.h
> @@ -73,7 +73,7 @@ struct _mesa_glsl_parse_state {
> _mesa_glsl_parse_state(struct gl_context *_ctx, GLenum target,
> void *mem_ctx);
>
> - DECLARE_RZALLOC_CXX_OPERATORS(_mesa_glsl_parse_state);
> + DECLARE_RALLOC_CXX_OPERATORS(_mesa_glsl_parse_state);
>
> /**
> * Generate a string representing the GLSL version currently being
> compiled
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
