On Tuesday, July 14, 2015 04:26:50 PM Timothy Arceri wrote:
> On Mon, 2015-07-13 at 22:19 -0700, Kenneth Graunke wrote:
> > On Monday, July 13, 2015 11:21:05 AM Iago Toral wrote:
> > > On Sat, 2015-07-11 at 10:13 +1000, Timothy Arceri wrote:
> > > > @@ -648,27 +653,28 @@ glsl_type::get_array_instance(const glsl_type
> > > > *base, unsigned array_size)
> > > > mtx_lock(&glsl_type::mutex);
> > > >
> > > > if (array_types == NULL) {
> > > > - array_types = hash_table_ctor(64, hash_table_string_hash,
> > > > - hash_table_string_compare);
> > > > + array_types = _mesa_hash_table_create(NULL,
> > > > _mesa_key_hash_string,
> > > > + _mesa_key_string_equal);
> > > > }
> > > >
> > > > - const glsl_type *t = (glsl_type *) hash_table_find(array_types,
> > > > key);
> > > > -
> > > > - if (t == NULL) {
> > > > + const struct hash_entry *entry =
> > > > _mesa_hash_table_search(array_types, key);
> > > > + if (entry == NULL) {
> > > > mtx_unlock(&glsl_type::mutex);
> > > > - t = new glsl_type(base, array_size);
> > > > + const glsl_type *t = new glsl_type(base, array_size);
> > > > mtx_lock(&glsl_type::mutex);
> > > >
> > > > - hash_table_insert(array_types, (void *) t,
> > > > ralloc_strdup(mem_ctx,
> > > > key));
> > > > + entry = _mesa_hash_table_insert(array_types,
> > > > + ralloc_strdup(mem_ctx, key),
> > > > + (void *) t);
> > > > }
> > > >
> > > > - assert(t->base_type == GLSL_TYPE_ARRAY);
> > > > - assert(t->length == array_size);
> > > > - assert(t->fields.array == base);
> > > > + assert(((glsl_type *)entry->data)->base_type == GLSL_TYPE_ARRAY);
> > > > + assert(((glsl_type *)entry->data)->length == array_size);
> > > > + assert(((glsl_type *)entry->data)->fields.array == base);
> > >
> > > Other parts of this file put a blank between the type cast and the
> > > variable, so I would add that here (and in all other places where you
> > > cast entry to glsl_type* in this patch).
> >
> > Or...why not continue to have a local variable t, and just set t =
> > entry->data? Then these could all stay the same, and there would be
> > less casting.
>
> I did have it like that but in my opinion it just looked messy. 2 extra lines
> vs 3 extra casts all of which are in asserts(), it felt like I was making the
> surrounding code worse for the sake of the asserts.
>
> I've pushed this already so I guess it doesn't matter now anyway.Yup, just an idea - what you had was fine. Thanks!
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
