the patch...

Bernhard Roider wrote:
Hello all,

today i searched for the reason why the vspace dialog is completely broken as Michael Gerz wrote in his mail from 27.02.2007. It turned out that the bug was introduced by the patch that fixed the bug that crashed lyx when "inset-insert ert" was executed from the command buffer.
The lexer has the method isOK() which reflects the status of the stream is.
The operators void* and ! are not really well defined (they depend on the value of is.bad()). What is missing is a test if the last reading operation was successful and thus the returned value is valid.
That's what i implemented in this patch.

The rule for using the lexer:

if you want to know if the lexer still has data to read (either from the stream or from the pushed token) then use "lex.isOK()". If you want to test if the last reading operation was successful then use eg. "if (lex) {...}" or unsuccessful then use eg. "if (!lex) {...}"

an example:

int readParam(LyxLex &lex) {

    int param = 1; // default value
    if (lex.isOK()) { // the lexer has data to read
        int p;    // temporary variable
        lex >> p;
if (lex) param = p; // only use the input if the reading operation was successful
    }
    return param;
}


I tested lyx with the attached patch and found no problems, but who knows...

i think this should go in.

comments?


Bernhard


PS: The methods LyXLex::Pimpl::next() and LyXLex::Pimpl::nextToken() read tokens in a really different way! is this intentionally?



Index: src/buffer.C
===================================================================
--- src/buffer.C        (revision 17394)
+++ src/buffer.C        (working copy)
@@ -649,7 +649,7 @@
        lex.next();
        string const token(lex.getString());
 
-       if (!lex.isOK()) {
+       if (!lex) {
                Alert::error(_("Document could not be read"),
                             bformat(_("%1$s could not be read."), 
from_utf8(filename.absFilename())));
                return failure;

Index: src/frontends/controllers/ControlLog.C
===================================================================
--- src/frontends/controllers/ControlLog.C      (revision 17394)
+++ src/frontends/controllers/ControlLog.C      (working copy)
@@ -43,7 +43,7 @@
 
        string logtype, logfile;
        lex >> logtype;
-       if (lex.isOK()) {
+       if (lex) {
                lex.next(true);
                logfile = lex.getString();
        }
Index: src/frontends/controllers/ControlParagraph.C
===================================================================
--- src/frontends/controllers/ControlParagraph.C        (revision 17394)
+++ src/frontends/controllers/ControlParagraph.C        (working copy)
@@ -53,7 +53,12 @@
                } else if (token == "update") {
                        lex.next();
                        bool const accept = lex.getBool();
-                       action = accept ? 1 : 2;
+                       if (lex) {
+                               action = accept ? 1 : 2;
+                       } else {
+                               // Unrecognised update option
+                               return false;
+                       }
                } else if (!token.empty()) {
                        // Unrecognised token
                        return false;
Index: src/insets/insetbox.C
===================================================================
--- src/insets/insetbox.C       (revision 17394)
+++ src/insets/insetbox.C       (working copy)
@@ -531,15 +531,17 @@
        if (!lex.isOK())
                return;
 
-       if (lex.isOK()) {
-               lex.next();
-               type = lex.getString();
-       }
-       if (!lex.isOK())
+       lex.next();
+       type = lex.getString();
+
+       if (!lex)
                return;
+
        lex.next();
        string token;
        token = lex.getString();
+       if (!lex)
+               return;
        if (token == "position") {
                lex.next();
                // The [0] is needed. We need the first and only char in
@@ -549,10 +551,11 @@
                lyxerr << "InsetBox::Read: Missing 'position'-tag!" << token << 
endl;
                lex.pushToken(token);
        }
-       if (!lex.isOK())
-               return;
+
        lex.next();
        token = lex.getString();
+       if (!lex)
+               return;
        if (token == "hor_pos") {
                lex.next();
                hor_pos = lex.getString()[0];
@@ -560,10 +563,11 @@
                lyxerr << "InsetBox::Read: Missing 'hor_pos'-tag!" << token << 
endl;
                lex.pushToken(token);
        }
-       if (!lex.isOK())
-               return;
+
        lex.next();
        token = lex.getString();
+       if (!lex)
+               return;
        if (token == "has_inner_box") {
                lex.next();
                inner_box = lex.getInteger();
@@ -572,10 +576,10 @@
                lex.pushToken(token);
        }
 
-       if (!lex.isOK())
-               return;
        lex.next();
        token = lex.getString();
+       if (!lex)
+               return;
        if (token == "inner_pos") {
                lex.next();
                inner_pos = lex.getString()[0];
@@ -584,10 +588,11 @@
                        << token << endl;
                lex.pushToken(token);
        }
-       if (!lex.isOK())
-               return;
+
        lex.next();
        token = lex.getString();
+       if (!lex)
+               return;
        if (token == "use_parbox") {
                lex.next();
                use_parbox = lex.getInteger();
@@ -595,10 +600,11 @@
                lyxerr << "InsetBox::Read: Missing 'use_parbox'-tag!" << endl;
                lex.pushToken(token);
        }
-       if (!lex.isOK())
-               return;
+
        lex.next();
        token = lex.getString();
+       if (!lex)
+               return;
        if (token == "width") {
                lex.next();
                width = LyXLength(lex.getString());
@@ -606,10 +612,11 @@
                lyxerr << "InsetBox::Read: Missing 'width'-tag!" << endl;
                lex.pushToken(token);
        }
-       if (!lex.isOK())
-               return;
+
        lex.next();
        token = lex.getString();
+       if (!lex)
+               return;
        if (token == "special") {
                lex.next();
                special = lex.getString();
@@ -617,10 +624,11 @@
                lyxerr << "InsetBox::Read: Missing 'special'-tag!" << endl;
                lex.pushToken(token);
        }
-       if (!lex.isOK())
-               return;
+
        lex.next();
        token = lex.getString();
+       if (!lex)
+               return;
        if (token == "height") {
                lex.next();
                height = LyXLength(lex.getString());
@@ -628,10 +636,11 @@
                lyxerr << "InsetBox::Read: Missing 'height'-tag!" << endl;
                lex.pushToken(token);
        }
-       if (!lex.isOK())
-               return;
+
        lex.next();
        token = lex.getString();
+       if (!lex)
+               return;
        if (token == "height_special") {
                lex.next();
                height_special = lex.getString();
Index: src/insets/insetert.C
===================================================================
--- src/insets/insetert.C       (revision 17394)
+++ src/insets/insetert.C       (working copy)
@@ -466,7 +466,7 @@
 
        int s;
        lex >> s;
-       if (lex.isOK())
+       if (lex)
                status = static_cast<InsetCollapsable::CollapseStatus>(s);
 }
 
Index: src/insets/insetnote.C
===================================================================
--- src/insets/insetnote.C      (revision 17394)
+++ src/insets/insetnote.C      (working copy)
@@ -105,7 +105,7 @@
 {
        string label;
        lex >> label;
-       if (lex.isOK())
+       if (lex)
                type = notetranslator().find(label);
 }
 
Index: src/insets/insettabular.C
===================================================================
--- src/insets/insettabular.C   (revision 17394)
+++ src/insets/insettabular.C   (working copy)
@@ -230,11 +230,11 @@
 
        lex.next();
        string token = lex.getString();
-       while (lex.isOK() && (token != "\\end_inset")) {
+       while (lex && token != "\\end_inset") {
                lex.next();
                token = lex.getString();
        }
-       if (token != "\\end_inset") {
+       if (!lex) {
                lex.printError("Missing \\end_inset at this point. "
                               "Read: `$$Token'");
        }
Index: src/insets/insetvspace.C
===================================================================
--- src/insets/insetvspace.C    (revision 17394)
+++ src/insets/insetvspace.C    (working copy)
@@ -88,7 +88,7 @@
        BOOST_ASSERT(lex.isOK());
        string vsp;
        lex >> vsp;
-       if (lex.isOK())
+       if (lex)
                space_ = VSpace(vsp);
 
        string end_token;
@@ -257,7 +257,7 @@
 
        string vsp;
        lex >> vsp;
-       if (lex.isOK())
+       if (lex)
                vspace = VSpace(vsp);
 }
 
Index: src/kbmap.C
===================================================================
--- src/kbmap.C (revision 17394)
+++ src/kbmap.C (working copy)
@@ -141,7 +141,7 @@
                        }
 
                        FuncRequest func = lyxaction.lookupFunc(cmd);
-                       if (func. action == LFUN_UNKNOWN_ACTION) {
+                       if (func.action == LFUN_UNKNOWN_ACTION) {
                                lexrc.printError("BN_BIND: Unknown LyX"
                                                 " function `$$Token'");
                                error = true;
Index: src/lyxlex.C
===================================================================
--- src/lyxlex.C        (revision 17394)
+++ src/lyxlex.C        (working copy)
@@ -53,7 +53,7 @@
 
 bool LyXLex::isOK() const
 {
-       return pimpl_->is.good();
+       return pimpl_->inputAvailable();
 }
 
 
@@ -122,37 +122,63 @@
 }
 
 
-int LyXLex::getInteger() const
+int LyXLex::getInteger()
 {
+       lastReadOk_ = pimpl_->status == LEX_DATA || pimpl_->status == LEX_TOKEN;
+       if (!lastReadOk_) {
+               pimpl_->printError("integer token missing");
+               return -1;
+       }
+
        if (isStrInt(pimpl_->getString()))
                return convert<int>(pimpl_->getString());
+
+       lastReadOk_ = false;
        pimpl_->printError("Bad integer `$$Token'");
        return -1;
 }
 
 
-double LyXLex::getFloat() const
+double LyXLex::getFloat()
 {
        // replace comma with dot in case the file was written with
        // the wrong locale (should be rare, but is easy enough to
        // avoid).
+       lastReadOk_ = pimpl_->status == LEX_DATA || pimpl_->status == LEX_TOKEN;
+       if (!lastReadOk_) {
+               pimpl_->printError("float token missing");
+               return -1;
+       }
+
        string const str = subst(pimpl_->getString(), ",", ".");
        if (isStrDbl(str))
                return convert<double>(str);
+
+       lastReadOk_ = false;
        pimpl_->printError("Bad float `$$Token'");
        return -1;
 }
 
 
-string const LyXLex::getString() const
+string const LyXLex::getString()
 {
+       lastReadOk_ = pimpl_->status == LEX_DATA || pimpl_->status == LEX_TOKEN;
+
+       if (lastReadOk_)
        return pimpl_->getString();
+
+       return string("");
 }
 
 
-docstring const LyXLex::getDocString() const
+docstring const LyXLex::getDocString()
 {
-       return pimpl_->getDocString();
+       lastReadOk_ = pimpl_->status == LEX_DATA || pimpl_->status == LEX_TOKEN;
+       
+       if (lastReadOk_)
+               return pimpl_->getDocString();
+
+       return docstring(from_utf8(""));
 }
 
 
@@ -164,7 +190,7 @@
        string str, prefix;
        bool firstline = true;
 
-       while (isOK()) {
+       while (pimpl_->is) { //< eatLine only reads from is, not from pushTok
                if (!eatLine())
                        // blank line in the file being read
                        continue;
@@ -197,7 +223,7 @@
                str += ltrim(tmpstr, "\t") + '\n';
        }
 
-       if (!isOK()) {
+       if (!pimpl_->is) {
                printError("Long string not ended by `" + endtoken + '\'');
        }
 
@@ -205,14 +231,17 @@
 }
 
 
-bool LyXLex::getBool() const
+bool LyXLex::getBool()
 {
        if (pimpl_->getString() == "true") {
+               lastReadOk_ = true;
                return true;
        } else if (pimpl_->getString() != "false") {
                pimpl_->printError("Bad boolean `$$Token'. "
                                   "Use \"false\" or \"true\"");
+               lastReadOk_ = false;
        }
+       lastReadOk_ = true;
        return false;
 }
 
@@ -246,13 +275,13 @@
        // use fail() here. However, our implementation of getString() et al.
        // can cause the eof() and fail() bits to be set, even though we
        // haven't tried to read 'em.
-       return pimpl_->is.bad() ? 0 : this;
+       return lastReadOk_? this: 0;
 }
 
 
 bool LyXLex::operator!() const
 {
-       return pimpl_->is.bad();
+       return !lastReadOk_;
 }
 
 
@@ -261,6 +290,8 @@
        if (isOK()) {
                next();
                s = getString();
+       } else {
+               lastReadOk_ = false;
        }
        return *this;
 }
@@ -271,6 +302,8 @@
        if (isOK()) {
                next();
                s = getDocString();
+       } else {
+               lastReadOk_ = false;
        }
        return *this;
 }
@@ -281,6 +314,8 @@
        if (isOK()) {
                next();
                s = getFloat();
+       } else {
+               lastReadOk_ = false;
        }
        return *this;
 }
@@ -291,6 +326,8 @@
        if (isOK()) {
                next();
                s = getInteger();
+       } else {
+               lastReadOk_ = false;
        }
        return *this;
 }
@@ -301,6 +338,8 @@
        if (isOK()) {
                next();
                s = getInteger();
+       } else {
+               lastReadOk_ = false;
        }
        return *this;
 }
@@ -311,6 +350,8 @@
        if (isOK()) {
                next();
                s = getBool();
+       } else {
+               lastReadOk_ = false;
        }
        return *this;
 }
Index: src/lyxlex.h
===================================================================
--- src/lyxlex.h        (revision 17394)
+++ src/lyxlex.h        (working copy)
@@ -101,16 +101,16 @@
        int getLineNo() const;
 
        ///
-       int getInteger() const;
+       int getInteger();
        ///
-       bool getBool() const;
+       bool getBool();
        ///
-       double getFloat() const;
+       double getFloat();
        ///
-       std::string const getString() const;
+       std::string const getString();
 
        ///
-       docstring const getDocString() const;
+       docstring const getDocString();
 
        /** Get a long string, ended by the tag `endtag'.
            This string can span several lines. The first line
@@ -162,6 +162,8 @@
        class Pimpl;
        ///
        Pimpl * pimpl_;
+       ///
+       bool lastReadOk_;
 };
 
 
Index: src/lyxlex_pimpl.C
===================================================================
--- src/lyxlex_pimpl.C  (revision 17394)
+++ src/lyxlex_pimpl.C  (working copy)
@@ -204,15 +204,15 @@
                // There can have been a whole line pushed so
                // we extract the first word and leaves the rest
                // in pushTok. (Lgb)
-               if (pushTok.find(' ') != string::npos && pushTok[0] == '\\') {
+               if (pushTok[0] == '\\' && pushTok.find(' ') != string::npos) {
                        buff.clear();
                        pushTok = split(pushTok, buff, ' ');
-                       return true;
                } else {
                        buff = pushTok;
                        pushTok.clear();
-                       return true;
                }
+               status = LEX_TOKEN;
+               return true;
        }
        if (!esc) {
                unsigned char c = 0; // getc() returns an int
@@ -308,24 +308,24 @@
                        // skip ','s
                        if (c == ',') continue;
 
-                       if (c == '\\') {
-                               // escape
-                               buff.clear();
+                       //if (c == '\\') { // covered by the later LEX_TOKEN 
part
+                       //      // escape
+                       //      buff.clear();
 
-                               do {
-                                       if (c == '\\') {
-                                               // escape the next char
-                                               is.get(cc);
-                                               c = cc;
-                                       }
-                                       buff.push_back(c);
-                                       is.get(cc);
-                                       c = cc;
-                               } while (c > ' ' && c != ',' && is);
+                       //      do {
+                       //              if (c == '\\') {
+                       //                      // escape the next char
+                       //                      is.get(cc);
+                       //                      c = cc;
+                       //              }
+                       //              buff.push_back(c);
+                       //              is.get(cc);
+                       //              c = cc;
+                       //      } while (c > ' ' && c != ',' && is);
 
-                               status = LEX_TOKEN;
-                               continue;
-                       }
+                       //      status = LEX_TOKEN;
+                       //      continue;
+                       //}
 
                        if (c == commentChar) {
                                // Read rest of line (fast :-)
@@ -453,10 +453,13 @@
        }
 
        if (c == '\n') {
+               buff.resize(buff.size() - 1);
                ++lineno;
-               buff.resize(buff.size() - 1);
                status = LEX_DATA;
                return true;
+       } else if (buff.length() > 0) { // last line
+               status = LEX_DATA;
+               return true;
        } else {
                return false;
        }
@@ -469,15 +472,15 @@
                // There can have been a whole line pushed so
                // we extract the first word and leaves the rest
                // in pushTok. (Lgb)
-               if (pushTok.find(' ') != string::npos && pushTok[0] == '\\') {
+               if (pushTok[0] == '\\' && pushTok.find(' ') != string::npos) {
                        buff.clear();
                        pushTok = split(pushTok, buff, ' ');
-                       return true;
                } else {
                        buff = pushTok;
                        pushTok.clear();
-                       return true;
                }
+               status = LEX_TOKEN;
+               return true;
        }
 
        status = 0;
@@ -519,6 +522,11 @@
 }
 
 
+bool LyXLex::Pimpl::inputAvailable() {
+       return pushTok.length() > 0 || is.good(); 
+}
+
+
 void LyXLex::Pimpl::pushToken(string const & pt)
 {
        pushTok = pt;
Index: src/lyxlex_pimpl.h
===================================================================
--- src/lyxlex_pimpl.h  (revision 17394)
+++ src/lyxlex_pimpl.h  (working copy)
@@ -65,6 +65,8 @@
        bool eatLine();
        ///
        bool nextToken();
+       /// test if there is a pushed token or the stream is ok
+       bool inputAvailable();
        ///
        void pushToken(std::string const &);
        /// fb_ is only used to open files, the stream is accessed through is.
Index: src/ToolbarBackend.C
===================================================================
--- src/ToolbarBackend.C        (revision 17394)
+++ src/ToolbarBackend.C        (working copy)
@@ -79,12 +79,12 @@
        Toolbar tb;
        tb.name = lex.getString();
        lex.next(true);
-       if (!lex.isOK()) {
+       tb.gui_name = lex.getString();
+       if (!lex) {
                lyxerr << "ToolbarBackend::read: Malformed toolbar "
                        "description " <<  lex.getString() << endl;
                return;
        }
-       tb.gui_name = lex.getString();
 
        bool quit = false;
 

Reply via email to