geda-developers team mailing list archive

[Krzysztof Kościuszkiewicz <k.kosciuszkiewicz@xxxxxxxxx>]

Re-sending to geda-devel to solicit comments...

Dnia 2012-01-30 o godzinie 01:00 Krzysztof Kościuszkiewicz napisał(a):
> Dnia 2012-01-30 o godzinie 01:58 Vladimir Zhbanov napisał(a):
> 
> > I've found gschem bug in git head. gschem segfaults when I close two
> > windows with unsaved pages.
> 
> ... 
> 
> > After bisecting I've got the following result:
> > 
> > 8342bddce4487edf4a7214d5d6ab83cb73a066d4 is the first bad commit
> 
> Thanks for the report and for finding the bad commit!  Long story
> short - the commit adds a "new toplevel" hook with hardcoded w_current
> pointing to the first gschem window.
> 
> When next window is created, hooks will still run with "old" w_current
> - it's a miracle this blows up only when gschem exits and frees the
> original w_current - but hooks for the subsequent windows still try to
> access it.

A patch for review is attached.  Ideas for better name/place for
gschem_toplevel_alloc_libgeda_toplevel() are welcome...

Best regards,
Krzysztof

>From 13d329ee5fb184c5dd73b0133722e9c5e32e0148 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Krzysztof=20Ko=C5=9Bciuszkiewicz?= <k.kosciuszkiewicz@xxxxxxxxx>
Date: Mon, 30 Jan 2012 23:32:34 +0100
Subject: [PATCH] gschem: fix segfault when multiple windows are closed

Reported by Vladimir Zhbanov <vzhbanov@xxxxxxxxx>,
introduced in commit 8342bddce4487edf4a7214d5d6ab83cb73a066d4.

Add new function gschem_toplevel_alloc_libgeda_toplevel which allocates
the TOPLEVEL and appends new hooks that force pin cues redraw when net
connectivity changes.

Previously each new TOPLEVEL had invalidation hooks attached which had
the GSCHEM_TOPLEVEL hardcoded, pointing at the very first window created
when gschem was launched.
---
 gschem/include/prototype.h   |    1 +
 gschem/src/gschem.c          |   17 +----------------
 gschem/src/gschem_toplevel.c |   10 ++++++++++
 gschem/src/i_callbacks.c     |    2 +-
 4 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/gschem/include/prototype.h b/gschem/include/prototype.h
index 42be9e0..640c497 100644
--- a/gschem/include/prototype.h
+++ b/gschem/include/prototype.h
@@ -2,6 +2,7 @@
 
 /* gschem_toplevel.c */
 GSCHEM_TOPLEVEL *gschem_toplevel_new();
+void gschem_toplevel_alloc_libgeda_toplevel(GSCHEM_TOPLEVEL *w_current);
 
 /* a_pan.c */
 void a_pan_general(GSCHEM_TOPLEVEL *w_current, double world_cx, double world_cy,
diff --git a/gschem/src/gschem.c b/gschem/src/gschem.c
index 1bb96c1..5f9b711 100644
--- a/gschem/src/gschem.c
+++ b/gschem/src/gschem.c
@@ -120,17 +120,6 @@ void gschem_quit(void)
   gtk_main_quit();
 }
 
-static void add_libgeda_toplevel_hooks (TOPLEVEL *toplevel,
-                                        GSCHEM_TOPLEVEL *w_current)
-{
-    o_attrib_append_attribs_changed_hook (toplevel,
-                                          (AttribsChangedFunc) o_invalidate,
-                                          w_current);
-    s_conn_append_conns_changed_hook (toplevel,
-                                      (ConnsChangedFunc) o_invalidate,
-                                      w_current);
-}
-
 /*! \brief Main Scheme(GUILE) program function.
  *  \par Function Description
  *  This function is the main program called from scm_boot_guile.
@@ -233,11 +222,7 @@ void main_prog(void *closure, int argc, char *argv[])
   /* Allocate w_current */
   w_current = gschem_toplevel_new ();
 
-  /* Connect hooks that run for each s_toplevel_new() first */
-  s_toplevel_append_new_hook ((NewToplevelFunc) add_libgeda_toplevel_hooks,
-                              w_current);
-
-  w_current->toplevel = s_toplevel_new ();
+  gschem_toplevel_alloc_libgeda_toplevel (w_current);
 
   w_current->toplevel->load_newer_backup_func = x_fileselect_load_backup;
   w_current->toplevel->load_newer_backup_data = w_current;
diff --git a/gschem/src/gschem_toplevel.c b/gschem/src/gschem_toplevel.c
index db97e29..983bbe4 100644
--- a/gschem/src/gschem_toplevel.c
+++ b/gschem/src/gschem_toplevel.c
@@ -208,3 +208,13 @@ GSCHEM_TOPLEVEL *gschem_toplevel_new ()
   return w_current;
 }
 
+void gschem_toplevel_alloc_libgeda_toplevel (GSCHEM_TOPLEVEL *w_current)
+{
+    w_current->toplevel = s_toplevel_new ();
+    o_attrib_append_attribs_changed_hook (w_current->toplevel,
+                                          (AttribsChangedFunc) o_invalidate,
+                                          w_current);
+    s_conn_append_conns_changed_hook (w_current->toplevel,
+                                      (ConnsChangedFunc) o_invalidate,
+                                      w_current);
+}
diff --git a/gschem/src/i_callbacks.c b/gschem/src/i_callbacks.c
index cc3a571..cebda89 100644
--- a/gschem/src/i_callbacks.c
+++ b/gschem/src/i_callbacks.c
@@ -104,7 +104,7 @@ DEFINE_I_CALLBACK(file_new_window)
   PAGE *page;
 
   w_current = gschem_toplevel_new ();
-  w_current->toplevel = s_toplevel_new ();
+  gschem_toplevel_alloc_libgeda_toplevel (w_current);
 
   w_current->toplevel->load_newer_backup_func = x_fileselect_load_backup;
   w_current->toplevel->load_newer_backup_data = w_current;
-- 
1.7.4.1