Re: [Mesa-dev] [PATCH] Fixed memory leak.

2013-11-27 Thread Siavash Eliasi 
Thank you very much Alex for the tip. I'll modify commits and send a v2 
series of patches. Before that I want to make sure that patch #17 is 
done correctly.



I'll be thankful if someone finds time to review and comment on this patch:

[Mesa-dev] [PATCH 17/17] Deleted gl_extensions::ARB_map_buffer_alignment 
and all of its use cases.

http://lists.freedesktop.org/archives/mesa-dev/2013-November/049047.html

Regards, Siavash Eliasi.

On 11/26/2013 10:26 PM, Alex Deucher wrote:

On Tue, Nov 26, 2013 at 1:22 PM, Siavash Eliasi siavashser...@gmail.com wrote:

In general, when you fix problems in prior patches, you should
integrate the fix into the original patch(es) where the problems were,
update the commit message to note what bugs were fixed and then
re-send the patch set.  That prevents broken commits from getting into
the git tree even if they are fixed in a later commit. You can use git
rebase -i to integrate your fixes.

Alex

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] Fixed memory leak.

2013-11-27 Thread Chris Forbes 
Patches 16, 17 are:

Reviewed-by: Chris Forbes chr...@ijw.co.nz

I'm not quite sure I buy the 4K alignment for i965 in patch 15 -- it's
true that any real BO mappings will be 4K-aligned, but when we have to
return a temporary chunk of memory this seems excessive.

Ian?

On Wed, Nov 27, 2013 at 11:26 PM, Siavash Eliasi
siavashser...@gmail.com wrote:
 Thank you very much Alex for the tip. I'll modify commits and send a v2
 series of patches. Before that I want to make sure that patch #17 is done
 correctly.


 I'll be thankful if someone finds time to review and comment on this patch:

 [Mesa-dev] [PATCH 17/17] Deleted gl_extensions::ARB_map_buffer_alignment and
 all of its use cases.
 http://lists.freedesktop.org/archives/mesa-dev/2013-November/049047.html

 Regards, Siavash Eliasi.


 On 11/26/2013 10:26 PM, Alex Deucher wrote:

 On Tue, Nov 26, 2013 at 1:22 PM, Siavash Eliasi siavashser...@gmail.com
 wrote:

 In general, when you fix problems in prior patches, you should
 integrate the fix into the original patch(es) where the problems were,
 update the commit message to note what bugs were fixed and then
 re-send the patch set.  That prevents broken commits from getting into
 the git tree even if they are fixed in a later commit. You can use git
 rebase -i to integrate your fixes.

 Alex

 ___
 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

[Mesa-dev] [PATCH] Fixed memory leak.

2013-11-26 Thread Siavash Eliasi 
---
 src/mesa/main/bufferobj.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
index 5581a5d..e68d96d 100644
--- a/src/mesa/main/bufferobj.c
+++ b/src/mesa/main/bufferobj.c
@@ -420,6 +420,8 @@ _mesa_buffer_data( struct gl_context *ctx, GLenum target, 
GLsizeiptrARB size,
 
new_data = _mesa_align_malloc( size, ctx-Const.MinMapBufferAlignment );
if (new_data) {
+  _mesa_align_free( bufObj-Data );
+
   bufObj-Data = (GLubyte *) new_data;
   bufObj-Size = size;
   bufObj-Usage = usage;
-- 
1.8.4.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] Fixed memory leak.

2013-11-26 Thread Alex Deucher 
On Tue, Nov 26, 2013 at 1:22 PM, Siavash Eliasi siavashser...@gmail.com wrote:

In general, when you fix problems in prior patches, you should
integrate the fix into the original patch(es) where the problems were,
update the commit message to note what bugs were fixed and then
re-send the patch set.  That prevents broken commits from getting into
the git tree even if they are fixed in a later commit. You can use git
rebase -i to integrate your fixes.

Alex

 ---
  src/mesa/main/bufferobj.c | 2 ++
  1 file changed, 2 insertions(+)

 diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
 index 5581a5d..e68d96d 100644
 --- a/src/mesa/main/bufferobj.c
 +++ b/src/mesa/main/bufferobj.c
 @@ -420,6 +420,8 @@ _mesa_buffer_data( struct gl_context *ctx, GLenum target, 
 GLsizeiptrARB size,

 new_data = _mesa_align_malloc( size, ctx-Const.MinMapBufferAlignment );
 if (new_data) {
 +  _mesa_align_free( bufObj-Data );
 +
bufObj-Data = (GLubyte *) new_data;
bufObj-Size = size;
bufObj-Usage = usage;
 --
 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] Fixed memory leak.

2013-11-26 Thread Siavash 

I'll fix this issue in v2 series of patches, thanks again!


Hi, I'm an outsider (for everyone who might have not noticed), I
started following the list because I'm interested in the newbie
projects, too.
I think this free should be called regardless of new_data being null,
as this doesn't imply there wasn't any in the original object.
So, I believe it should be:

if( bufObj-Data )
   _mesa_align_free( bufObj-Data );

if( new_data ) {
   bufObj-Data = new_data;
...


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev