Merge lp:~vanvugt/compiz/fix-1097664 into lp:compiz/0.9.9

Proposed by Daniel van Vugt on 2013-01-09
Status: Merged
Approved by: Sam Spilsbury on 2013-01-09
Approved revision: 3550
Merged at revision: 3552
Proposed branch: lp:~vanvugt/compiz/fix-1097664
Merge into: lp:compiz/0.9.9
Diff against target: 40 lines (+7/-1)
3 files modified
plugins/opengl/include/opengl/opengl.h (+1/-1)
plugins/opengl/include/opengl/shadercache.h (+1/-0)
plugins/opengl/src/shadercache.cpp (+5/-0)
To merge this branch: bzr merge lp:~vanvugt/compiz/fix-1097664
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve on 2013-01-09
Sam Spilsbury 2013-01-09 Approve on 2013-01-09
Review via email: mp+142471@code.launchpad.net

Commit message

Avoid leaking GLShaderCache::priv (LP: #1097664)

I have bumped the opengl ABI for completeness but it's probably not necessary
because:
  (a) No code outside of the opengl plugin uses GLShaderCache; and
  (b) Even if there was code using it, I'm not sure if the addition of a
      non-virtual destructor would affect the binary object size.

To post a comment you must log in.
Sam Spilsbury (smspillaz) wrote :

I don't think adding a destructor will break the ABI, as one is generated automatically. Its only if you are overloading a virtual destructor, or adding one.

In any case, the best way to handle this problem is to use either unique_ptr or auto_ptr for the pimpl. I know, templates are evil in the eyes of some. Its not that evil and saves time if someone accidentally removes the appropriate delete from the destructor one day.

review: Approve
Sam Spilsbury (smspillaz) wrote :

s/adding one/adding a virtual destructor/.

PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/opengl/include/opengl/opengl.h'
2--- plugins/opengl/include/opengl/opengl.h 2013-01-04 07:49:07 +0000
3+++ plugins/opengl/include/opengl/opengl.h 2013-01-09 10:50:25 +0000
4@@ -50,7 +50,7 @@
5 #include <opengl/programcache.h>
6 #include <opengl/shadercache.h>
7
8-#define COMPIZ_OPENGL_ABI 6
9+#define COMPIZ_OPENGL_ABI 7
10
11 /*
12 * Some plugins check for #ifdef USE_MODERN_COMPIZ_GL. Support it for now, but
13
14=== modified file 'plugins/opengl/include/opengl/shadercache.h'
15--- plugins/opengl/include/opengl/shadercache.h 2012-05-17 10:41:21 +0000
16+++ plugins/opengl/include/opengl/shadercache.h 2013-01-09 10:50:25 +0000
17@@ -83,6 +83,7 @@
18 {
19 public:
20 GLShaderCache ();
21+ ~GLShaderCache ();
22
23 /**
24 * Gets the GLShaderData associated with the specified parameters.
25
26=== modified file 'plugins/opengl/src/shadercache.cpp'
27--- plugins/opengl/src/shadercache.cpp 2012-05-17 10:41:21 +0000
28+++ plugins/opengl/src/shadercache.cpp 2013-01-09 10:50:25 +0000
29@@ -100,6 +100,11 @@
30 {
31 }
32
33+GLShaderCache::~GLShaderCache ()
34+{
35+ delete priv;
36+}
37+
38 const GLShaderData &
39 GLShaderCache::getShaderData (const GLShaderParameters &params)
40 {

Subscribers

People subscribed via source and target branches