[Mesa-dev] [PATCH 7/8] glsl: Replace iterators in ir_reader.cpp with ad-hoc list walking.

[Ian Romanick]

On 01/11/2014 02:37 AM, Kenneth Graunke wrote:
> These can't use foreach_list since they want to skip over the first few
> list elements.  Just doing the ad-hoc list walking isn't too bad.
> 
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/glsl/ir_reader.cpp | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/src/glsl/ir_reader.cpp b/src/glsl/ir_reader.cpp
> index f5185d2..28923f3 100644
> --- a/src/glsl/ir_reader.cpp
> +++ b/src/glsl/ir_reader.cpp
> @@ -205,11 +205,12 @@ ir_reader::read_function(s_expression *expr, bool skip_body)
>        assert(added);
>     }
>  
> -   exec_list_iterator it = ((s_list *) expr)->subexpressions.iterator();
> -   it.next(); // skip "function" tag
> -   it.next(); // skip function name
> -   for (/* nothing */; it.has_next(); it.next()) {
> -      s_expression *s_sig = (s_expression *) it.get();
> +   /* Skip over "function" tag and function name (which are guaranteed to be
> +    * present by the above PARTIAL_MATCH call).
> +    */
> +   exec_node *node = ((s_list *) expr)->subexpressions.head->next->next;
> +   for (/* nothing */; !node->is_tail_sentinel(); node = node->next) {
> +      s_expression *s_sig = (s_expression *) node;

This won't behave the same in the (bug) case that the list has too few
elements.  If the list is empty or as only one element, there will be a
NULL deref here somewhere.  I believe the iterator version was safe
against this.

Do we have some pre-existing guarantee that the list has enough elements?

>        read_function_sig(f, s_sig, skip_body);
>     }
>     return added ? f : NULL;
> @@ -249,9 +250,10 @@ ir_reader::read_function_sig(ir_function *f, s_expression *expr, bool skip_body)
>     exec_list hir_parameters;
>     state->symbols->push_scope();
>  
> -   exec_list_iterator it = paramlist->subexpressions.iterator();
> -   for (it.next() /* skip "parameters" */; it.has_next(); it.next()) {
> -      ir_variable *var = read_declaration((s_expression *) it.get());
> +   /* Skip over the "parameters" tag. */
> +   exec_node *node = paramlist->subexpressions.head->next;
> +   for (/* nothing */; !node->is_tail_sentinel(); node = node->next) {
> +      ir_variable *var = read_declaration((s_expression *) node);
>        if (var == NULL)
>  	 return;
>  
>