Browse Source

[2375] Remove exception handling from getNextToken

It was deemed unneeded and is removed therefore, to make the code
smaller.
Michal 'vorner' Vaner 12 years ago
parent
commit
0c0888ef6a

+ 11 - 21
src/lib/dns/master_lexer.cc

@@ -170,34 +170,24 @@ MasterLexer::getNextToken(Options options) {
     if (impl_->source_ == NULL || impl_->source_->atEOF()) {
     if (impl_->source_ == NULL || impl_->source_->atEOF()) {
         isc_throw(isc::InvalidOperation, "No source to read tokens from");
         isc_throw(isc::InvalidOperation, "No source to read tokens from");
     }
     }
-    // Store current state, for the case we need to restore by ungetToken
-    impl_->has_previous_ = false;
-    // If thrown in this section, nothing observable except for not having
-    // previous will happen.
+    // Store the current state so we can restore it in ungetToken
     impl_->previous_paren_count_ = impl_->paren_count_;
     impl_->previous_paren_count_ = impl_->paren_count_;
     impl_->previous_was_eol_ = impl_->last_was_eol_;
     impl_->previous_was_eol_ = impl_->last_was_eol_;
     impl_->source_->mark();
     impl_->source_->mark();
     impl_->has_previous_ = true;
     impl_->has_previous_ = true;
     // Reset the token now. This is to check a token was actually produced.
     // Reset the token now. This is to check a token was actually produced.
     // This is debugging aid.
     // This is debugging aid.
-
-    // From now on, if we throw, we can at least restore the old state.
-    try {
-        impl_->token_ = Token(Token::NO_TOKEN_PRODUCED);
-        for (const State *state = start(options); state != NULL;
-             state = state->handle(*this)) {
-            // Do nothing here. All is handled in the for cycle header itself.
-        }
-        // Make sure a token was produced. Since this Can Not Happen, we assert
-        // here instead of throwing.
-        assert(impl_->token_.getType() != Token::ERROR ||
-               impl_->token_.getErrorCode() != Token::NO_TOKEN_PRODUCED);
-        return (impl_->token_);
-    } catch (...) {
-        // Restore the old state and let the exception continue.
-        ungetToken();
-        throw;
+    impl_->token_ = Token(Token::NO_TOKEN_PRODUCED);
+    // And get the token
+    for (const State *state = start(options); state != NULL;
+         state = state->handle(*this)) {
+        // Do nothing here. All is handled in the for cycle header itself.
     }
     }
+    // Make sure a token was produced. Since this Can Not Happen, we assert
+    // here instead of throwing.
+    assert(impl_->token_.getType() != Token::ERROR ||
+           impl_->token_.getErrorCode() != Token::NO_TOKEN_PRODUCED);
+    return (impl_->token_);
 }
 }
 
 
 void
 void

+ 5 - 5
src/lib/dns/master_lexer.h

@@ -191,11 +191,11 @@ public:
     /// It reads a bit of the last opened source and produces another token
     /// It reads a bit of the last opened source and produces another token
     /// found in it.
     /// found in it.
     ///
     ///
-    /// This method provides the weak exception guarantee. It'll return the
-    /// class to the state before the call on exception, except that it is
-    /// not possible to call ungetToken() after the failed call. This is
-    /// considered OK, since the caller was expecting the call to succeed
-    /// in which case the previous token would not be ungettable as well.
+    /// This method does not provide the strong exception guarantee. Generally,
+    /// if it throws, the object should not be used any more and should be
+    /// discarded. It was decided all the exceptions thrown from here are
+    /// serious enough that aborting the loading process is the only reasonable
+    /// recovery anyway, so the strong exception guarantee is not needed.
     ///
     ///
     /// \param options The options can be used to modify the tokenization.
     /// \param options The options can be used to modify the tokenization.
     ///     The method can be made reporting things which are usually ignored
     ///     The method can be made reporting things which are usually ignored

+ 0 - 34
src/lib/dns/tests/master_lexer_unittest.cc

@@ -393,38 +393,4 @@ TEST_F(MasterLexerTest, ungetAfterSwitch) {
     EXPECT_THROW(lexer.ungetToken(), isc::InvalidOperation);
     EXPECT_THROW(lexer.ungetToken(), isc::InvalidOperation);
 }
 }
 
 
-class TestException {};
-
-void
-doThrow(const std::string&) {
-    throw TestException();
-}
-
-// Check the getNextToken provides at least the weak exception guarantee.
-TEST_F(MasterLexerTest, getTokenExceptions) {
-    ss << "\n12345";
-    lexer.pushSource(ss);
-
-    // Prepare a chain that changes the internal state, reads something.
-    // The next item in the chain will throw an exception (we explicitly
-    // throw something not known to it, so we know it can handle anything).
-    // Then the thing should get to the previous state and getting the
-    // token the usual way without mock should work.
-    const bool true_value = true;
-    boost::scoped_ptr<State> s2(State::getFakeState(NULL, 3, NULL, 0, NULL,
-                                                    &doThrow));
-    boost::scoped_ptr<State> s1(State::getFakeState(s2.get(), 3, NULL, 1,
-                                                    &true_value));
-    lexer.pushFakeStart(s1.get());
-
-    // Getting the token with the fake start should throw. But then, the
-    // current state should be untouched.
-    EXPECT_THROW(lexer.getNextToken(), TestException);
-    EXPECT_EQ(0, s1->getParenCount(lexer));
-    EXPECT_FALSE(s1->wasLastEOL(lexer));
-
-    // It gets back to the original state, so getting the newline works.
-    EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType());
-}
-
 }
 }