Qucs / qucs

Qucs Project official mirror
http://qucs.sourceforge.net/
GNU General Public License v2.0
1.15k stars 213 forks source link

BPF_1550_edge_cpld.sch example crashes Qucs #150

Closed in3otd closed 9 years ago

in3otd commented 9 years ago

Opening the example circuit examples/BPF_1550_edge_cpld.sch makes Qucs crash. This seems somehow related to having diagram together with the schematic. Offending commit appears to be 33224a17cca9bbc3c5725bba34cf115882d8af7c

guitorri commented 9 years ago

Part of the crash report:

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   org.qucs                        0x00000001012678ec Q3GList::count() const + 12 (q3glist.h:160)
1   org.qucs                        0x00000001016b9d15 Q3PtrList<DataX>::isEmpty() const + 21 (q3ptrlist.h:88)
2   org.qucs                        0x00000001016b7893 TabDiagram::calcDiagram() + 2995 (tabdiagram.cpp:179)
3   org.qucs                        0x00000001016a6fab Diagram::updateGraphData() + 43 (diagram.cpp:810)
4   org.qucs                        0x00000001016a51ca Diagram::loadGraphData(QString const&) + 986 (diagram.cpp:772)
5   org.qucs                        0x0000000101280ba2 Schematic::reloadGraphs() + 210 (schematic.cpp:1282)
6   org.qucs                        0x000000010127f8e0 Schematic::becomeCurrent(bool) + 1808 (schematic.cpp:245)
7   org.qucs                        0x0000000101280cdc non-virtual thunk to Schematic::becomeCurrent(bool) + 44 (schematic.cpp:247)
8   org.qucs                        0x00000001012a4fe8 QucsApp::slotChangeView(QWidget*) + 456 (qucs.cpp:1909)
9   org.qucs                        0x00000001012971c9 QucsApp::gotoPage(QString const&) + 1241 (qucs.cpp:1524)

Note that if you run with 0.0.18 (just to generate the result file) and try to open again with master it loads fine. There is something about the TabDiagram that is failing to ignore the missing data.

guitorri commented 9 years ago

I think we saw this issue already: ad1ac33

in3otd commented 9 years ago

yes, and the fix was almost fine... :wink:

just saw that in your commit for all the diagrams the code that was like

while(g->cPointsX.isEmpty()) {  // any graph with data ?
  g = Graphs.next();
  if(g == 0) break;
 }

was changed to

while(g->cPointsX.isEmpty()) {  // any graph with data ?
  g = ig.next();
  if(!ig.hasNext()) break;
}

is this correct? When there is just one graph, exiting the loop g points to some random address (it's not necessarily null), so following tests like if(g) or if(!g->cPointsX.isEmpty()) might not give the expected results... Shouldn't this be

while(g->cPointsX.isEmpty()) {
  if (!ig.hasNext()) break; // no more graphs
  g = ig.next(); // point to next graph
}

and then in the following code check again g->cPointsX.isEmpty() to see if we should do something? Or at least with this changes it seems to work.

Note that the current code also crashes when there was a timingdiagram with just one variable and no data were available.

in3otd commented 9 years ago

oups, clicked the wrong button...