Re: [Mesa-dev] [PATCH 11/17] Modified _mesa_buffer_data to use _mesa_align_malloc.
On Sun, Nov 24, 2013 at 11:36 PM, Siavash Eliasi wrote: > --- > src/mesa/main/bufferobj.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c > index b27f592..5581a5d 100644 > --- a/src/mesa/main/bufferobj.c > +++ b/src/mesa/main/bufferobj.c > @@ -416,9 +416,9 @@ _mesa_buffer_data( struct gl_context *ctx, GLenum target, > GLsizeiptrARB size, > { > void * new_data; > > - (void) ctx; (void) target; > + (void) target; > > - new_data = _mesa_realloc( bufObj->Data, bufObj->Size, size ); > + new_data = _mesa_align_malloc( size, ctx->Const.MinMapBufferAlignment ); realloc can be used to either allocate a new buffer or re-allocate an existing buffer (which may or may not return a different pointer). Given that, I think that this will leak the old buffer (if one has already been allocated). From looking at the rest of the surrounding code in the patch, we don't preserve the existing buffer contents, so I guess I'd just FREE(bufObj->Data) within the following if block before assigning bufObj->Data = newdata; --Aaron > if (new_data) { >bufObj->Data = (GLubyte *) new_data; >bufObj->Size = size; > -- > 1.8.4.2 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 11/17] Modified _mesa_buffer_data to use _mesa_align_malloc.
Yes I think you are right, I guess *_mesa_align_realloc* is the correct function which should be used here. What do you think Ian? Best regards, Siavash Eliasi. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 11/17] Modified _mesa_buffer_data to use _mesa_align_malloc.
Fixed memory leak, thanks for the heads up! http://lists.freedesktop.org/archives/mesa-dev/2013-November/049178.html Best Regards, Siavash Eliasi. On 11/26/2013 01:14 AM, Aaron Watry wrote: On Sun, Nov 24, 2013 at 11:36 PM, Siavash Eliasi wrote: --- src/mesa/main/bufferobj.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c index b27f592..5581a5d 100644 --- a/src/mesa/main/bufferobj.c +++ b/src/mesa/main/bufferobj.c @@ -416,9 +416,9 @@ _mesa_buffer_data( struct gl_context *ctx, GLenum target, GLsizeiptrARB size, { void * new_data; - (void) ctx; (void) target; + (void) target; - new_data = _mesa_realloc( bufObj->Data, bufObj->Size, size ); + new_data = _mesa_align_malloc( size, ctx->Const.MinMapBufferAlignment ); realloc can be used to either allocate a new buffer or re-allocate an existing buffer (which may or may not return a different pointer). Given that, I think that this will leak the old buffer (if one has already been allocated). From looking at the rest of the surrounding code in the patch, we don't preserve the existing buffer contents, so I guess I'd just FREE(bufObj->Data) within the following if block before assigning bufObj->Data = newdata; --Aaron if (new_data) { bufObj->Data = (GLubyte *) new_data; bufObj->Size = size; -- 1.8.4.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 11/17] Modified _mesa_buffer_data to use _mesa_align_malloc.
Now I understand why _mesa_realloc is not appropriate to use here. From spec: glBufferData creates a new data store for the buffer object currently bound to target. Any pre-existing data store is deleted. The new data store is created with the specified size in bytes and usage. If data is NULL, a data store of the specified size is still created, but its contents remain uninitialized and thus undefined. Best regards, Siavash Eliasi. On 11/26/2013 09:34 AM, Siavash Eliasi wrote: Yes I think you are right, I guess *_mesa_align_realloc* is the correct function which should be used here. What do you think Ian? Best regards, Siavash Eliasi. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 11/17] Modified _mesa_buffer_data to use _mesa_align_malloc.
On Tue, Nov 26, 2013 at 11:34 AM, Siavash Eliasi wrote: > Now I understand why _mesa_realloc is not appropriate to use here. From > spec: > > glBufferData creates a new data store for the buffer object currently >> bound to target. Any pre-existing data store is deleted. The new data store >> is created with the specified size in bytes and usage. >> If data is NULL, a data store of the specified size is still created, but >> its contents remain uninitialized and thus undefined. >> > > > Best regards, > Siavash Eliasi. > > > On 11/26/2013 09:34 AM, Siavash Eliasi wrote: > >> Yes I think you are right, I guess *_mesa_align_realloc* is the correct >> function which should be used here. >> >> What do you think Ian? >> >> >> Best regards, >> Siavash Eliasi. >> > Realloc was a poor choice in the first place. It would needlessly copy data from the old buffer to the new one. -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev