On 27 October 2013 14:59, Ian Romanick <i...@freedesktop.org> wrote:
> From: Ian Romanick <ian.d.roman...@intel.com>
>
> I made this a function (instead of a method of ir_variable) because it
> made the change set smaller, and I expect that there will be an overload
> that takes an ir_var_mode enum. Having both functions used the same way
> seemed better.
>
> v2: Add missing case for ir_var_system_value.
>
> Signed-off-by: Ian Romanick <ian.d.roman...@intel.com>
> ---
> src/glsl/ir.cpp | 41 +++++++++++++++++++++++++++++++++++++++++
> src/glsl/ir.h | 3 +++
> src/glsl/linker.cpp | 23 -----------------------
> 3 files changed, 44 insertions(+), 23 deletions(-)
>
> diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp
> index c682e3e..90e8f5d 100644
> --- a/src/glsl/ir.cpp
> +++ b/src/glsl/ir.cpp
> @@ -1891,3 +1891,44 @@ vertices_per_prim(GLenum prim)
> return 3;
> }
> }
> +
> +/**
> + * Generate a string describing the mode of a variable
> + */
> +const char *
> +mode_string(const ir_variable *var)
> +{
> + switch (var->mode) {
> + case ir_var_auto:
> + return (var->read_only) ? "global constant" : "global variable";
> +
> + case ir_var_uniform:
> + return "uniform";
> +
> + case ir_var_shader_in:
> + return "shader input";
> +
> + case ir_var_shader_out:
> + return "shader output";
> +
> + case ir_var_function_in:
> + case ir_var_const_in:
> + return "function input";
> +
> + case ir_var_function_out:
> + return "function output";
> +
> + case ir_var_function_inout:
> + return "function inout";
> +
> + case ir_var_system_value:
> + return "shader input";
> +
> + case ir_var_temporary:
> + return "compiler temporary";
> +
> + case ir_var_mode_count:
> + assert(!"Should not get here.");
> + return "invalid variable";
>
I'd suggest changing this case to "break;", and moving these two lines
after the end of the switch statement. That way, in the unlikely event
that var->mode is an invalid value other than ir_var_mode_count, the
assertion will still fire, and in release builds we won't wind up returning
a garbage pointer.
With that change, the patch is:
Reviewed-by: Paul Berry <stereotype...@gmail.com>
> + }
> +}
> diff --git a/src/glsl/ir.h b/src/glsl/ir.h
> index 8d5bec9..75c9f18 100644
> --- a/src/glsl/ir.h
> +++ b/src/glsl/ir.h
> @@ -2292,6 +2292,9 @@ extern char *
> prototype_string(const glsl_type *return_type, const char *name,
> exec_list *parameters);
>
> +const char *
> +mode_string(const ir_variable *var);
> +
> extern "C" {
> #endif /* __cplusplus */
>
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index d8f655c..d081afb 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -558,29 +558,6 @@ validate_geometry_shader_executable(struct
> gl_shader_program *prog,
>
>
> /**
> - * Generate a string describing the mode of a variable
> - */
> -static const char *
> -mode_string(const ir_variable *var)
> -{
> - switch (var->mode) {
> - case ir_var_auto:
> - return (var->read_only) ? "global constant" : "global variable";
> -
> - case ir_var_uniform: return "uniform";
> - case ir_var_shader_in: return "shader input";
> - case ir_var_shader_out: return "shader output";
> -
> - case ir_var_const_in:
> - case ir_var_temporary:
> - default:
> - assert(!"Should not get here.");
> - return "invalid variable";
> - }
> -}
> -
> -
> -/**
> * Perform validation of global variables used across multiple shaders
> */
> void
> --
> 1.8.1.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
