Re: [Mesa-dev] [PATCH 11/17] Modified _mesa_buffer_data to use _mesa_align_malloc.

2013-11-27 Thread Brian Paul 
On Tue, Nov 26, 2013 at 11:34 AM, Siavash Eliasi siavashser...@gmail.comwrote:

 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

Re: [Mesa-dev] [PATCH 11/17] Modified _mesa_buffer_data to use _mesa_align_malloc.

2013-11-26 Thread Siavash Eliasi 

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
siavashser...@gmail.com 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.

2013-11-26 Thread Siavash Eliasi 
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.

2013-11-25 Thread Aaron Watry 
On Sun, Nov 24, 2013 at 11:36 PM, Siavash Eliasi
siavashser...@gmail.com 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.

2013-11-25 Thread Siavash Eliasi 
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

[Mesa-dev] [PATCH 11/17] Modified _mesa_buffer_data to use _mesa_align_malloc.

2013-11-24 Thread Siavash Eliasi 
---
 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 );
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