On 02/10/2014 03:43 PM, Brian Paul wrote:
> Fix formatting, add new comments, get rid of extraneous indentation.
> Suggested by Ian in bug 74723.
> ---
> src/mesa/main/shaderapi.c | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
> index 97a57a4..44b4c3a 100644
> --- a/src/mesa/main/shaderapi.c
> +++ b/src/mesa/main/shaderapi.c
> @@ -383,31 +383,31 @@ detach_shader(struct gl_context *ctx, GLuint program,
> GLuint shader)
> _mesa_reference_shader(ctx, &shProg->Shaders[i], NULL);
>
> /* alloc new, smaller array */
I was looking at this code earlier today. Would this be better?
/* Move the "tail" of the list of shaders up by one element.
*/
memmove(&shProg->Shaders[j], &shProg->Shaders[j + 1],
sizeof(shProg->Shaders[0]) * (n - (j + 1)));
/* Compact the array. Not worried about compacting realloc failures.
*/
newList = realloc(shProg->Shaders,
sizeof(shProg->Shaders[0]) * (n - 1));
if (!newList) {
_mesa_error(ctx, GL_OUT_OF_MEMORY, "glDetachShader");
return;
}
shProg->Shaders = newList;
shProg->NumShaders = n - 1;
For that matter, do we even need to compact the allocation?
/* Move the "tail" of the list of shaders up by one element.
*/
memmove(&shProg->Shaders[j], &shProg->Shaders[j + 1],
sizeof(shProg->Shaders[0]) * (n - (j + 1)));
shProg->Shaders[n - 1] = 0;
shProg->NumShaders = n - 1;
Whichever way you feel like doing it, they're all improvements.
Reviewed-by: Ian Romanick <ian.d.roman...@intel.com>
> - newList =
> - malloc((n - 1) * sizeof(struct gl_shader *));
> + newList = malloc((n - 1) * sizeof(struct gl_shader *));
> if (!newList) {
> _mesa_error(ctx, GL_OUT_OF_MEMORY, "glDetachShader");
> return;
> }
> + /* Copy old list entries to new list, skipping removed entry at [i]
> */
> for (j = 0; j < i; j++) {
> newList[j] = shProg->Shaders[j];
> }
> - while (++i < n)
> + while (++i < n) {
> newList[j++] = shProg->Shaders[i];
> - free(shProg->Shaders);
> + }
>
> + /* Free old list and install new one */
> + free(shProg->Shaders);
> shProg->Shaders = newList;
> shProg->NumShaders = n - 1;
>
> #ifdef DEBUG
> - /* sanity check */
> - {
> - for (j = 0; j < shProg->NumShaders; j++) {
> - assert(shProg->Shaders[j]->Type == GL_VERTEX_SHADER ||
> - shProg->Shaders[j]->Type == GL_GEOMETRY_SHADER ||
> - shProg->Shaders[j]->Type == GL_FRAGMENT_SHADER);
> - assert(shProg->Shaders[j]->RefCount > 0);
> - }
> + /* sanity check - make sure the new list's entries are sensible */
> + for (j = 0; j < shProg->NumShaders; j++) {
> + assert(shProg->Shaders[j]->Type == GL_VERTEX_SHADER ||
> + shProg->Shaders[j]->Type == GL_GEOMETRY_SHADER ||
> + shProg->Shaders[j]->Type == GL_FRAGMENT_SHADER);
> + assert(shProg->Shaders[j]->RefCount > 0);
> }
> #endif
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
