On 01/13/2014 09:49 AM, Ian Romanick wrote: > 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 <kenn...@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?
Yes. Above: s_pattern pat[] = { "function", name }; if (!PARTIAL_MATCH(expr, pat)) { ir_read_error(expr, "Expected (function <name> (signature ...) ...)"); return NULL; } If the list doesn't match the (partial) S-Expression (function <name> ...) we would have bailed by now. So the list is guaranteed to have at least two 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; >> >> > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev