On Thu, Apr 5, 2018 at 10:06 AM, Caio Marcelo de Oliveira Filho <
caio.olive...@intel.com> wrote:
> Hello,
>
> > + if (count <= max_short_path_len) {
> > + /* If we're under max_short_path_len, just use the short path. */
> > + path->path = head;
> > + goto done;
> > + }
> > +
> > + path->path = ralloc_array(mem_ctx, nir_deref_instr *, count + 1);
> > + head = tail = path->path + count;
> > + *tail = NULL;
> > + for (nir_deref_instr *d = deref; d; d = nir_deref_instr_parent(d))
> > + *(--head) = d;
>
> What do you think about zeroing the _short_path when we are not using
> it? I'm guessing cases where we don't use _short_path will not be
> common, so it will help highlight them when debugging.
>
We could. We could also memset it to 0xdeadbeef in debug builds which is
probably better than NULL.
> > +done:
> > + assert(head == path->path);
> > + assert(tail == head + count);
> > + assert((*head)->deref_type == nir_deref_type_var);
>
> This assert access invalid memory if "deref == NULL", but the rest of
> the code is ready for this case. So I suggest either prefixing this
> assert with "(!*head) || ..." or, if the deref == NULL case is not
> useful/expected, assert(deref) in the beginning of the function.
>
It does. However, I think it should be an error to call this function with
NULL (I can add an assert) and all of the callers assume that the first
path.path[0] will be a variable dereference so we really do want to check
that.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
