Merge lp:~ghugesss/xpad/xpad into lp:xpad

Proposed by Sagar Ghuge
Status: Merged
Approved by: Arthur Borsboom
Approved revision: no longer in the source branch.
Merged at revision: 699
Proposed branch: lp:~ghugesss/xpad/xpad
Merge into: lp:xpad
Diff against target: 56 lines (+4/-8)
3 files modified
ChangeLog (+1/-0)
src/xpad-app.c (+3/-7)
src/xpad-pad-group.c (+0/-1)
To merge this branch: bzr merge lp:~ghugesss/xpad/xpad
Reviewer Review Type Date Requested Status
Arthur Borsboom quick code test Approve
Review via email: mp+227890@code.launchpad.net

Description of the change

Fix seg fault :

We are trying to unref this : gtk_icon_theme_get_default (void) but doc says that we should not ref it and unref it.

checked with object ref count every thing looks fine. No other ref remains.

To post a comment you must log in.
Revision history for this message
Arthur Borsboom (arthurborsboom) wrote :

Hi Sagar,

Most of the offered code looks good and might indeed solve the problem.
What is the reason you take out this piece of code?

group->priv->pads = NULL;

Revision history for this message
Sagar Ghuge (ghugesss) wrote :

Hi,

If I am not wrong then pads are actually our toplevel widgets and we are destroying it. so if we destroy the toplevel widget according to documentation those will gets finalized automatically so I don't think so that we need to set pads to NULL. That's why I removed the code. And if I introduce that line again then it will give me the seg fault. ( as it is already finalized and still we were trying to set it to NULL ) that's why may be.

lp:~ghugesss/xpad/xpad updated
696. By Launchpad Translations on behalf of xpad-team

Launchpad automatic translations update.

697. By Launchpad Translations on behalf of xpad-team

Launchpad automatic translations update.

698. By Launchpad Translations on behalf of xpad-team

Launchpad automatic translations update.

Revision history for this message
Arthur Borsboom (arthurborsboom) wrote :

Code looks good.

I did a quick test: run xpad, check preferences, start/stop, etc.
I didn't do a refcount check. We will do this before releasing.

Good job!

review: Approve (quick code test)
lp:~ghugesss/xpad/xpad updated
699. By Arthur Borsboom

* Fix: Seg Fault - Seg fault on closing the xpad (#1333727)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'ChangeLog'
--- ChangeLog 2014-07-20 16:36:07 +0000
+++ ChangeLog 2014-07-23 11:11:38 +0000
@@ -1,5 +1,6 @@
1Version 4.41Version 4.4
2* Fix: Preferences - redesign of preferences box to make sure it fits on small screens (#1333636)2* Fix: Preferences - redesign of preferences box to make sure it fits on small screens (#1333636)
3* Fix: Seg Fault - Seg fault on closing the xpad (#1333727)
34
4Version 4.35Version 4.3
5* New: Systray - ability to hide the tray icon (#890334) 6* New: Systray - ability to hide the tray icon (#890334)
67
=== modified file 'src/xpad-app.c'
--- src/xpad-app.c 2014-06-19 19:33:52 +0000
+++ src/xpad-app.c 2014-07-23 11:11:38 +0000
@@ -245,15 +245,13 @@
245void245void
246xpad_app_quit (void)246xpad_app_quit (void)
247{247{
248 /* Give GTK the signal to clean the rest and quit the application. */
249 gtk_main_quit ();
250
251 /* Free the memory used by the pads belonging to this group */248 /* Free the memory used by the pads belonging to this group */
252 xpad_pad_group_destroy_pads (pad_group);249 xpad_pad_group_destroy_pads (pad_group);
253250
254 /* Free the memory used by group. */251 /* Free the memory used by group. */
255 if (G_IS_OBJECT (pad_group))252 if (G_IS_OBJECT (pad_group))
256 g_object_unref (pad_group);253 g_object_unref (pad_group);
254 pad_group = NULL;
257255
258 /* Free the memory used by the tray icon and its menu. */256 /* Free the memory used by the tray icon and its menu. */
259 xpad_tray_dispose (settings);257 xpad_tray_dispose (settings);
@@ -263,10 +261,8 @@
263 g_object_unref (settings);261 g_object_unref (settings);
264 settings = NULL; /* This is needed due to the asynchronous finalizing process. */262 settings = NULL; /* This is needed due to the asynchronous finalizing process. */
265263
266 /* Free the theme reference. Unfortunately GTK3 leaves about 600 objects behind. */264 /* Give GTK the signal to clean the rest and quit the application. */
267 if (G_IS_OBJECT (gtk_icon_theme_get_default ()))265 gtk_main_quit ();
268 g_object_unref (gtk_icon_theme_get_default ());
269
270}266}
271267
272static gboolean268static gboolean
273269
=== modified file 'src/xpad-pad-group.c'
--- src/xpad-pad-group.c 2014-06-20 10:23:13 +0000
+++ src/xpad-pad-group.c 2014-07-23 11:11:38 +0000
@@ -124,7 +124,6 @@
124 xpad_pad_group_save_unsaved_all(group);124 xpad_pad_group_save_unsaved_all(group);
125125
126 g_slist_foreach (group->priv->pads, (GFunc) gtk_widget_destroy, NULL);126 g_slist_foreach (group->priv->pads, (GFunc) gtk_widget_destroy, NULL);
127 group->priv->pads = NULL;
128}127}
129128
130guint129guint

Subscribers

People subscribed via source and target branches