Hi Tim, In general we could use the odd local variable to make things shorter (and cut down the number of derefs). That can (should?) be done once we're finished with the big churn.
On 11 November 2016 at 00:45, Timothy Arceri <timothy.arc...@collabora.com> wrote: > @@ -296,6 +296,8 @@ init_shader_program(struct gl_shader_program *prog) > prog->Type = GL_SHADER_PROGRAM_MESA; > prog->RefCount = 1; > > + prog->data = create_shader_program_data(); > + This can fail. Please move it a level up (such that it's symmetric to the dtor) and call ralloc_free(shProg) on failure. > prog->AttributeBindings = string_to_uint_map_ctor(); > prog->FragDataBindings = string_to_uint_map_ctor(); > prog->FragDataIndexBindings = string_to_uint_map_ctor(); > @@ -309,7 +311,7 @@ init_shader_program(struct gl_shader_program *prog) > > exec_list_make_empty(&prog->EmptyUniformLocations); > > - prog->InfoLog = ralloc_strdup(prog, ""); > + prog->data->InfoLog = ralloc_strdup(prog->data, ""); IMHO it's fine keeping this piece here (despite the above suggestion). > + _mesa_uniform_detach_all_driver_storage(&shProg->data-> > + UniformStorage[i]); Hmm had no idea this is legal. Wondering how many compilers will be happy with it - worth keeping on single line ? Please check if we leak due to the missing ctx in create_shader_program_data() and if we're fine _do_ ignore my suggestion. With the small nitpicks patches 12-15 incl. are Reviewed-by: Emil Velikov <emil.veli...@collabora.com> -Emil _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
