tags 474313 patch
thanks
On Sun, Jul 13, 2008 at 07:31:57PM +0200, Georges Khaznadar wrote:
> Hello, I tried to reproduce the bug about a 100 µF capacitor, the way
> you described it:
>
> > To reproduce, start ktechlab, create a new circuit, drag a capacitor
> > onto it, select the capacitor, then change its value in the top bar.
>
> and the bug was not triggered. I am using the version 0.3-9 of ktechlab,
> with an amd64 platform.
Strange that it's so different on amd64 - after looking more closely, I was
able to identify two distinct bugs in the code.
I retried with 0.3-9 inside a sid chroot on my i386 machine. First of all,
this version *crashed* when I dragged any symbol onto a new circuit. I
recompiled with debugging symbols and -fno-inline using gcc 4.3.1, and got:
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0xb5e7b700 (LWP 32442)]
0x082455fd in Map::reset (this=0x8941480) at matrix.cpp:308
308 (*m_map)[i][j] = 0;
(gdb) bt
#0 0x082455fd in Map::reset (this=0x8941480) at matrix.cpp:308
#1 0x082466ab in Map (this=0x8941480, size=2) at matrix.cpp:292
#2 0x08246776 in Matrix (this=0x893b958, n=2, m=0) at matrix.cpp:35
#3 0x0824220d in ElementSet (this=0x8941920, circuit=0x893ab88, n=2, m=0) at
elementset.cpp:32
#4 0x0823f77f in Circuit::init (this=0x893ab88) at circuit.cpp:203
...
It turned out that Map::Map() creates a "new ETMap(m_size)" and then calls
reset(), which assumes that *m_map is square. However, the "new
ETMap(m_size)" only creates a vector that contains two *empty* vectors, so
reset() would overwrite random memory. The attached patch fixes this by not
calling reset() and instead initializing the array properly.
Recommendation to upstream authors: Use valgrind!
OK, this allowed me to actually trigger the bug that I originally reported,
and which still existed. It turned out to be infinite recursion:
#135 0x0814ec5b in DoubleSpinBox::valueChanged (this=0xa34ccb0,
t0=9.9999999999999991e-05) at doublespinbox.moc:95
#136 0x0814f63a in DoubleSpinBox::checkIfChanged (this=0xbf7d4aac) at
doublespinbox.cpp:210
#137 0x0814f7c8 in DoubleSpinBox::qt_invoke (this=0xa34ccb0, _id=56,
_o=0xbf7d4bdc) at doublespinbox.moc:103
#138 0xb6582f6d in QObject::activate_signal () from /usr/lib/libqt-mt.so.3
#139 0xb65839f0 in QObject::activate_signal () from /usr/lib/libqt-mt.so.3
#140 0xb68c7960 in QSpinBox::valueChanged () from /usr/lib/libqt-mt.so.3
#141 0xb669a4c6 in QSpinBox::valueChange () from /usr/lib/libqt-mt.so.3
#142 0xb668cd59 in QRangeControl::setValue () from /usr/lib/libqt-mt.so.3
#143 0xb6698b23 in QSpinBox::setValue () from /usr/lib/libqt-mt.so.3
#144 0xb669a02b in QSpinBox::interpretText () from /usr/lib/libqt-mt.so.3
#145 0xb6698385 in QSpinBox::value () from /usr/lib/libqt-mt.so.3
#146 0xb6699987 in QSpinBox::currentValueText () from /usr/lib/libqt-mt.so.3
#147 0xb6699bfa in QSpinBox::selectAll () from /usr/lib/libqt-mt.so.3
#148 0xb669a2fb in QSpinBox::updateDisplay () from /usr/lib/libqt-mt.so.3
#149 0xb669a4a8 in QSpinBox::valueChange () from /usr/lib/libqt-mt.so.3
#150 0xb668cd59 in QRangeControl::setValue () from /usr/lib/libqt-mt.so.3
#151 0xb6698b23 in QSpinBox::setValue () from /usr/lib/libqt-mt.so.3
#152 0x0814fb23 in DoubleSpinBox::setValue (this=0xa34ccb0,
value=9.9999999999999991e-05) at doublespinbox.cpp:78
#153 0x080ca642 in ItemInterface::slotSetData (this=0xa17dad0, [EMAIL
PROTECTED], value={d = 0xbf7d4fcc}) at iteminterface.cpp:570
#154 0x080cab29 in ItemInterface::tbDataChanged (this=0xa17dad0) at
iteminterface.cpp:480
#155 0x080cc668 in ItemInterface::qt_invoke (this=0xa17dad0, _id=4,
_o=0xbf7d50bc) at iteminterface.moc:107
#156 0xb6582f6d in QObject::activate_signal () from /usr/lib/libqt-mt.so.3
#157 0xb658388d in QObject::activate_signal () from /usr/lib/libqt-mt.so.3
#158 0x0814ec5b in DoubleSpinBox::valueChanged (this=0xa34ccb0,
t0=9.9999999999999991e-05) at doublespinbox.moc:95
The first and last line of the above are the same: The code recurses
forever updating the widget. This is because in
DoubleSpinBox::checkIfChanged(), the check "m_lastEmittedValue == newValue"
never becomes true due to a rounding error.
The bug is that the comparison takes place using a double value (e.g.
100.0e-6 for 100uF), whereas the widget stores two items, an integer 100
and a multiplier (like 1.0e-6 for micro). Converting the two back and forth
never gives an exact match.
(Incidentally, I find it strange that the widget needs to emit "I have
changed" in response to it receiving a "you have changed". It smells wrong,
but I don't understand the code well enough to say whether it's a bug.)
My fix is to perform the above comparison on the two values (integer and
multiplier) rather than the combined double. The patch is attached. I've
only tested it lightly!
Cheers,
Richard
--
__ _
|_) /| Richard Atterer
| \/¯| http://atterer.net
¯ '` ¯
--- ./src/gui/doublespinbox.cpp.orig 2008-07-13 20:52:20.247138183 +0000
+++ ./src/gui/doublespinbox.cpp 2008-07-13 21:57:12.163137261 +0000
@@ -33,7 +33,8 @@
DoubleSpinBox::DoubleSpinBox( double lower, double upper, double minAbs,
double value, const QString &unit, QWidget * parent )
: QSpinBox( parent )
{
- m_lastEmittedValue = value;
+ m_lastEmittedNumber = 0;
+ m_lastEmittedMult = 0.0;
m_unit = unit;
m_minValue = lower;
m_maxValue = upper;
@@ -201,13 +202,14 @@
void DoubleSpinBox::checkIfChanged()
{
- double newValue = value();
-
- if ( m_lastEmittedValue == newValue )
+ int num = getDisplayedNumber( 0 );
+ double mult = getMult();
+ if ( m_lastEmittedNumber == num && m_lastEmittedMult == mult )
return;
+ m_lastEmittedNumber = num;
+ m_lastEmittedMult = mult;
- m_lastEmittedValue = newValue;
- emit valueChanged( m_lastEmittedValue );
+ emit valueChanged( value() );
}
--- ./src/gui/doublespinbox.h.orig 2008-07-13 22:20:10.855136297 +0000
+++ ./src/gui/doublespinbox.h 2008-07-13 21:49:19.571137553 +0000
@@ -125,7 +125,8 @@
double m_minValue;
double m_maxValue;
double m_minAbsValue;
- double m_lastEmittedValue;
+ int m_lastEmittedNumber;
+ double m_lastEmittedMult;
};
#endif
--- ./src/electronics/simulation/matrix.cpp.orig 2008-07-13
20:32:07.715136621 +0000
+++ ./src/electronics/simulation/matrix.cpp 2008-07-13 20:44:40.143134992
+0000
@@ -288,8 +288,15 @@
Map::Map( const uint size )
{
m_size = size;
- m_map = new ETMap( m_size );
- reset();
+ m_map = new ETMap(m_size);
+ for ( uint i = 0; i<m_size; i++)
+ {
+ std::vector<uint>* v = &(*m_map)[i];
+ for ( uint j=0; j<m_size; j++ )
+ {
+ v->push_back(0);
+ }
+ }
}
--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
