Bug 135354 - crash when equation contains [scalar][scalar]
Summary: crash when equation contains [scalar][scalar]
Status: RESOLVED FIXED
Alias: None
Product: kst
Classification: Applications
Component: general (show other bugs)
Version: 1.x
Platform: unspecified Solaris
: NOR crash
Target Milestone: ---
Assignee: kst
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-10-09 18:06 UTC by Nicolas Brisset
Modified: 2006-10-16 18:57 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Nicolas Brisset 2006-10-09 18:06:18 UTC
Version:           1.3.1_devel (using KDE 3.4.0, compiled sources)
Compiler:          gcc version 3.4.3
OS:                SunOS (sun4u) release 5.8

There is a problem in the equation parser, which leads to a crash. To reproduce:
1) load column 1 vs INDEX from gyrodata.dat
2) create an equation with [1]+[C_PI][C_PI] and "INDEX" as X vector
3) Hit "OK": crash!

It should either recognize [C_PI][C_PI] as [C_PI]*[C_PI] and do it correctly, or invalidate the syntax.
The way [1][1] is handled leaves me quite puzzled :-( But it does not seem to be very clean, as kst crashes upon exit (or even before) even though it is less obvious than the previous case.
Comment 1 Adam Treat 2006-10-09 18:28:19 UTC
Couple of comments...

1. An equation which references multiple scalars is indeed giving a bogus error that it can't find the scalar the second time.  That is easy to fix.

2. [1] is not an existing scalar.  Kst will interpret this as a reference to a scalar object found in the global list of scalars.  If it doesn't currently exist, it will throw an error.  You can check if it is listed in the 'View Scalars' dialog to be sure.

3.  Is the implicit multiply operator supposed to work?  It definitely shouldn't crash, but throw a parse error I'd imagine.
Comment 2 Nicolas Brisset 2006-10-09 18:38:53 UTC
In that case, [1] is the name of a vector (the first column from gyrodata.dat, the example file in misc/tutorial).
Comment 3 Adam Treat 2006-10-09 19:02:49 UTC
Ok, then #3 is the only relevant question and I've found a EParseErrorNoImplicitMultiply in the parser.  So, this bug is really why the parser isn't throwing this error instead of crashing.
Comment 4 George Staikos 2006-10-09 20:42:17 UTC
  The error isn't used because the implementation of it caused a regression 
elsewhere.
Comment 5 George Staikos 2006-10-09 20:42:26 UTC
We don't support implicit multiplication and don't have plans to at this time.
Comment 6 Adam Treat 2006-10-16 18:10:20 UTC
This is not a problem as implicit multiplication is not allowed.  The testcases for the equation parser already note this.
Comment 7 Nicolas Brisset 2006-10-16 18:42:59 UTC
Not a problem functionality-wise, but I still find it bad that users may lose work because kst crashes before they get a chance to save ! Especially as the way scalars need to be edited makes this very likely to happen !
Is it complicated to make sure that:
a) the syntax is rejected 
b) if it is accepted (as seems to be the case right now) returned values are NaN (indicating a problem) and kst does not crash ?
Comment 8 George Staikos 2006-10-16 18:55:56 UTC
Right, and I see why it crashes.
Comment 9 George Staikos 2006-10-16 18:57:42 UTC
SVN commit 596053 by staikos:

don't crash when an equation fails to parse
BUG: 135354


 M  +1 -1      kstequation.cpp  


--- trunk/extragear/graphics/kst/src/libkstmath/kstequation.cpp #596052:596053
@@ -211,7 +211,7 @@
   Equation::Context ctx;
   ctx.sampleCount = _ns;
   ctx.xVector = v;
-  usedUpdated = KstObject::UPDATE == _pe->update(update_counter, &ctx);
+  usedUpdated = _pe && KstObject::UPDATE == _pe->update(update_counter, &ctx);
 
   KstObject::UpdateType rc = NO_CHANGE; // if force, rc = UPDATE anyway.
   if (force || xUpdated || usedUpdated) {