--===============1959601895882693818== Content-Type: multipart/alternative; boundary="===============7963513154959930416==" --===============7963513154959930416== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103182/#review8882 ----------------------------------------------------------- I am not sure (3) makes sense. Also would it be possible for you to send tw= o separate patches, one with just the spacing and the other with the featur= e? It would make it really easier to review, since now you have to look twi= ce to see if it's just whitespace changes or also there some code change in= a given line. Sorry for taking so much to answer but kmplot development is basically stal= led and i just found your review request while doing a rourinary "janitor" = review request round. - Albert Astals Cid On Nov. 18, 2011, 11:26 p.m., Eike Krumbacher wrote: > = > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/103182/ > ----------------------------------------------------------- > = > (Updated Nov. 18, 2011, 11:26 p.m.) > = > = > Review request for KDE Edu. > = > = > Description > ------- > = > Hello Folks! > = > If you have a set of functions like > f(x) =3D g x=C2=B2 > g(x) =3D 2 x > = > * kmplot crashes without notice, because the parser reads "g" as a functi= on and reports "Wrong number of arguments" but crashes. > * If "g" is meant to be a predefined constant the parser crashes because = g is read as a function with wrong argument counts. = > = > So there are several cases: > 1) g(x) and f(x) exists in a file, reading the file should not crash > 2) if g is a constant, whether in the file or "global", f(x) should read = this constant first > 3) a function set like = > g(x) =3D 10 * g > f(x) =3D g(x) + g > should be read and displayed. > = > With the given diff, we can archive (1) and (2) but not (3), which now do= es not crash anymore but gives strange results. In the surrounding of the = changes, I added some whitespaces to reflect the style guide of the given c= ode. The main part is an introduction of the token "ERROR", adding this tok= en to the stack in case of an error. = > = > Have fun > = > Eike > = > = > This addresses bug 204068. > http://bugs.kde.org/show_bug.cgi?id=3D204068 > = > = > Diffs > ----- > = > kmplot/parser.h c3fb92e = > kmplot/parser.cpp 23569aa = > = > Diff: http://git.reviewboard.kde.org/r/103182/diff/diff > = > = > Testing > ------- > = > (1), (2) and (3) was tested. The original file from the bug report associ= ated with this review was tested. No crashes in these special cases. = > = > = > Thanks, > = > Eike Krumbacher > = > --===============7963513154959930416== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable
This is an automatically generated e-mail. To reply, visit: http://git.revie= wboard.kde.org/r/103182/ 

I am not s=
ure (3) makes sense. Also would it be possible for you to send two separate=
 patches, one with just the spacing and the other with the feature? It woul=
d make it really easier to review, since now you have to look twice to see =
if it's just whitespace changes or also there some code change in a giv=
en line.

Sorry for taking so much to answer but kmplot development is basically stal=
led and i just found your review request while doing a rourinary "jani=
tor" review request round.

- Albert


On November 18th, 2011, 11:26 p.m., Eike Krumbacher wrote:

Review request for KDE Edu.
By Eike Krumbacher.

Updated Nov. 18, 2011, 11:26 p.m.

Descripti= on 

Hello Folks!

If you have a set of functions like
f(x) =3D g x=C2=B2
g(x) =3D 2 x

* kmplot crashes without notice, because the parser reads "g" as =
a function and reports "Wrong number of arguments" but crashes.
* If "g" is meant to be a predefined constant the parser crashes =
because g is read as a function with wrong argument counts. =


So there are several cases:
1) g(x) and f(x) exists in a file, reading the file should not crash
2) if g is a constant, whether in the file or "global", f(x) shou=
ld read this constant first
3) a function set like =

g(x) =3D 10 * g
f(x) =3D g(x) + g
should be read and displayed.

With the given diff, we can archive (1) and (2) but not (3), which now does=
 not crash anymore but  gives strange results. In the surrounding of the ch=
anges, I added some whitespaces to reflect the style guide of the given cod=
e. The main part is an introduction of the token "ERROR", adding =
this token to the stack in case of an error. =


Have fun

Eike

Testing <= /h1> 
(1), (2) and (3) was tested. The original file from the bug =
report associated with this review was tested. No crashes in these special =
cases.
Bugs: 204068

Diffs=

  • kmplot/parser.h (c3fb92e)
  • kmplot/parser.cpp (23569aa)

View Diff
--===============7963513154959930416==-- --===============1959601895882693818== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ kde-edu mailing list kde-edu@mail.kde.org https://mail.kde.org/mailman/listinfo/kde-edu --===============1959601895882693818==--