Exception usage cleanup. Use exceptions to terminate the parser in all cases.

Also protect pointers with auto_ptr to ensure cleanup.

Note: Before this change exceptions *could* occur, but all pointer-referenced
memory would be leaked.

* tparse/tparse.h: Change #include-s to C++ names.
  (RCSParseError):  Change to be a public descendant of exception. Use
  string datatype to ensure automatic cleanup. Implement a destructor
  as required by exception.
  (RCSExpected):  Use string datatype to ensure automatic cleanup. Add
  constructor for expected characters.
  (Branche): Removed.  Replaced with std::list<>.
  (Sink): Change method definition to reflect switch to exceptions.
  (TokenParser::match):  Add method for matching characters.
  (TokenParser::semicol, TokenParser::matchsemicol): Removed.
  Obsolete because we're now able to match single characters.
  (tparseParser):  Change method definition to reflect the switch
  to exceptions.
  (tparseParser::parse):  Adapt to changed method definitions.

* tparse/tparse.cpp: Change #include-s to C++ names.
  (TokenParser::get): Space changes and conversion to auto_ptr.
  (tparseParser::parse_rcs_admin,
   tparseParser::parse_rcs_description,
   tparseParser::parse_rcs_deltatext): Conversion to auto_ptr and exceptions.
  (tparseParser::parse_rcs_tree): Conversion to auto_ptr and exceptions. Also
  adjust for removal of Branche class.

* tparse/tparsemodule.h:  Include Python.h, since
  we,re actually using python types.

* tparse/tparsemodule.cpp (pyobject, pystring):
  New classes.  Used to anchor python objects ensuring their refcounts
  are decremented.
  (chkpy):  New.  Function to check python return value and act appropriately.
  (initparse):  Correctly anchor python strings.
  (rcstoken_to_pystring):  Removed.  Now integrated into the pystring class.
  (PythonSink):  Claim ownership for the duration of the instance lifetime.
  Also adjust all methods for the switch to exceptions.
  (tparse):  Adapt to the switch to exceptions.  Also prevent memory leakage
  when an exception occurs.


git-svn-id: http://viewvc.tigris.org/svn/viewvc/trunk@1060 8cb11bc2-c004-0410-86c3-e597b4017df7

tparse/tparse.cpp

@@ -31,10 +31,14 @@
 */
 
 #include "tparse.h"
+
 #ifndef __USE_XOPEN
 #define __USE_XOPEN
 #endif
-#include <time.h>
+#include <ctime>   /* for strptime */
+
+
+using namespace std;
 
 #define Whitespace(c) (c == ' ' || c == '\t' || c == '\014' || c == '\n' || \
                        c == '\r')
@@ -90,22 +94,25 @@ rcstoken *rcstoken::copy_begin_len(size_t begin, size_t len)
 /*--------- Tokenparser class -----------*/
 rcstoken *TokenParser::get()
 {
-  rcstoken *token;
+  auto_ptr<rcstoken> token;
 
   if (backget)
   {
-    token = backget;
+    token.reset(backget);
     backget = NULL;
-    return token;
+
+    return token.release();
   }
 
+  token.reset(new rcstoken());
   while (1)
   {
     if (idx == buflength)
     {
       input->read(buf, CHUNK_SIZE);
       if ( (buflength = input->gcount()) == 0 )
-        return NULL;
+          return token.release();
+
       idx = 0;
     }
     if (!Whitespace(buf[idx]))
@@ -113,17 +120,17 @@ rcstoken *TokenParser::get()
     idx++;
   }
 
-  token = new rcstoken();
   if (buf[idx] == ';')
   {
     idx++;
     (*token) = ';';
-    return token;
+    return token.release();
   }
 
   if (buf[idx] != '@')
   {
     int end = idx + 1;
+
     while (1)
     {
       while ( (end < buflength) && !(Token_term(buf[end])) )
@@ -132,7 +139,7 @@ rcstoken *TokenParser::get()
       if (end < buflength)
       {
         idx = end;
-        return token;
+        return token.release();
       }
       input->read(buf, CHUNK_SIZE);
       buflength = input->gcount();
@@ -141,9 +148,11 @@ rcstoken *TokenParser::get()
     }
   }
   idx++;
+
   while (1)
   {
     int i;
+
     if (idx == buflength)
     {
       idx = 0;
@@ -181,11 +190,10 @@ rcstoken *TokenParser::get()
     if ((i - idx) > 0)
       token->append(buf + idx, i - idx);
     idx = i + 1;
-    return token;
+    return token.release();
   }
 };
 
-
 void TokenParser::unget(rcstoken *token)
 {
   if (backget)
@@ -197,84 +205,63 @@ void TokenParser::unget(rcstoken *token)
 }
 
 /*--------- tparseParser class -----------*/
-int tparseParser::parse_rcs_admin()
+void tparseParser::parse_rcs_admin()
 {
   while (1)
   {
-    rcstoken *token = tokenstream->get();
+    auto_ptr<rcstoken> token(tokenstream->get());
+
     if (isdigit((*token)[0]))
     {
-      tokenstream->unget(token);
+      tokenstream->unget(token.release());
-      return 0;
+      return;
     }
     if (*token == "head")
     {
-      delete token;
+      token.reset(tokenstream->get());
-      if (sink->set_head_revision(token = tokenstream->get()))
+      sink->set_head_revision(*token);
-      {
+
-        delete token;
+      tokenstream->match(';');
-        return 1;
-      }
-      tokenstream->matchsemicol();
-      delete token;
       continue;
     }
     if (*token == "branch")
     {
-      rcstoken *branch = tokenstream->get();
+      token.reset(tokenstream->get());
-      if (*branch != ';')
+      if (*token != ';')
       {
-        if (sink->set_principal_branch(branch))
+        sink->set_principal_branch(*token);
-        {
+
-          delete branch;
+        tokenstream->match(';');
-          delete token;
-          return 1;
-        }
-        delete branch;
-        tokenstream->matchsemicol();
       }
-      delete token;
       continue;
     }
     if (*token == "symbols")
     {
       while (1)
       {
-        rcstoken *tag, *rev;
+        auto_ptr<rcstoken> tag, rev;
         char *second;
-        delete token;
+        //        delete token;
-        token = tokenstream->get();
+        token.reset(tokenstream->get());
         if (*token == ';')
           break;
 
         /*FIXME: this does not allow "<tag> : <rev>"
           which the spec does allow */
         second = index(token->data, ':');
-        tag = token->copy_begin_len(0, second - token->data);
+        tag.reset(token->copy_begin_len(0, second - token->data));
         second++;
-        rev = new rcstoken(second);
+        rev.reset(new rcstoken(second));
-        if (sink->define_tag(tag, rev))
+        sink->define_tag(*tag, *rev);
-        {
-          delete tag;
-          delete rev;
-          delete token;
-          return 1;
-        }
-        delete tag;
-        delete rev;
       }
       continue;
     }
     if (*token == "comment")
     {
-      delete token;
+      token.reset(tokenstream->get());
-      if (sink->set_comment(token = tokenstream->get()))
+      sink->set_comment((*token));
-      {
+
-        delete token;
+      tokenstream->match(';');
-        return 1;
-      }
-      tokenstream->matchsemicol();
-      delete token;
       continue;
     }
     if (*token == "locks" ||
@@ -284,81 +271,79 @@ int tparseParser::parse_rcs_admin()
     {
       while (1)
       {
-        rcstoken *tag = tokenstream->get();
+        token.reset(tokenstream->get());
-        if (*tag == ';')
+        if (*token == ';')
-          {
+          break;
-            delete tag;
-            break;
-          }
-        delete tag;
       }
-      delete token;
       continue;
     }
-    delete token;
   }
 };
 
-int tparseParser::parse_rcs_tree()
+void tparseParser::parse_rcs_tree()
 {
   while (1)
   {
-    rcstoken *revision;
+    auto_ptr<rcstoken> revision, date, author, hstate, next;
-    rcstoken *date;
     long timestamp;
-    rcstoken *author;
+    tokenlist branches;
-    rcstoken *hstate;
-    rcstoken *next;
-    Branche *branches = NULL;
     struct tm tm;
-    revision = tokenstream->get();
+
+    revision.reset(tokenstream->get());
     if (*revision == "desc")
     {
-      tokenstream->unget(revision);
+      tokenstream->unget(revision.release());
-      return 0;
+      return;
     }
+
     // Parse date
     tokenstream->match("date");
-    date = tokenstream->get();
+    date.reset(tokenstream->get());
-    tokenstream->matchsemicol();
+    tokenstream->match(";");
+
     memset ((void *) &tm, 0, sizeof(struct tm));
-    if (strptime(date->data, "%y.%m.%d.%H.%M.%S", &tm) == NULL)
+    if (strptime((*date).data, "%y.%m.%d.%H.%M.%S", &tm) == NULL)
-      strptime(date->data, "%Y.%m.%d.%H.%M.%S", &tm);
+      strptime((*date).data, "%Y.%m.%d.%H.%M.%S", &tm);
     timestamp = mktime(&tm);
-    delete date;
+
+
     tokenstream->match("author");
-    author = tokenstream->get();
+    author.reset(tokenstream->get());
-    tokenstream->matchsemicol();
+    tokenstream->match(';');
+
     tokenstream->match("state");
-    hstate = new rcstoken();
+    hstate.reset(new rcstoken());
     while (1)
     {
-      rcstoken *token = tokenstream->get();
+      auto_ptr<rcstoken> token;
-      if (*token == ';')
+      token.reset(tokenstream->get());
-      {
-        break;
-      }
-      if (hstate->length)
-        (*hstate) += ' ';
-      (*hstate) += *token;
-      delete token;
-    }
-    tokenstream->match("branches");
-    while (1)
-    {
-      rcstoken *token = tokenstream->get();
       if (*token == ';')
         break;
 
-      branches = new Branche(token, branches);
+      if ((*hstate).length)
+        (*hstate) += ' ';
+      (*hstate) += *token;
     }
+
+    tokenstream->match("branches");
+    while (1)
+      {
+        auto_ptr<rcstoken> token;
+        token.reset(tokenstream->get());
+        if (*token == ';')
+          break;
+
+        branches.push_front((*token));
+      }
+
     tokenstream->match("next");
-    next = tokenstream->get();
+    next.reset(tokenstream->get());
     if (*next == ';')
       /* generate null token */
-      next = new rcstoken();
+      next.reset(new rcstoken());
     else
-      tokenstream->matchsemicol();
+      tokenstream->match(';');
+
     /*
      * 	there are some files with extra tags in them. for example:
      *	owner	640;
@@ -367,77 +352,53 @@ int tparseParser::parse_rcs_tree()
      *	hardlinks	@configure.in@;
      *	this is "newphrase" in RCSFILE(5). we just want to skip over these.
      */
-
     while (1)
-    {
-      rcstoken *token = tokenstream->get();
-      if (*token == "desc" || isdigit((*token)[0]))
       {
-        tokenstream->unget(token);
+        auto_ptr<rcstoken> token;
-        break;
+        token.reset(tokenstream->get());
-      };
-      delete token;
-      while ( (*(token = tokenstream->get())) == ';')
-        delete token;
-    }
 
-    if (sink->define_revision(revision, timestamp, author,
+        if ((*token == "desc") || isdigit((*token)[0]) )
-                              hstate, branches, next))
+          {
-      {
+            tokenstream->unget(token.release());
-        delete revision;
+            break;
-        delete author;
+          };
-        delete hstate;
+
-        delete branches;
+        while (*token != ";")
-        delete next;
+          token.reset(tokenstream->get());
-        return 1;
       }
-    delete revision;
+
-    delete author;
+    sink->define_revision(*revision, timestamp, *author,
-    delete hstate;
+                          *hstate, branches, *next);
-    delete branches;
-    delete next;
   }
-  return 0;
+  return;
 }
 
-int tparseParser::parse_rcs_description()
+void tparseParser::parse_rcs_description()
 {
-  rcstoken *token;
+  auto_ptr<rcstoken> token;
   tokenstream->match("desc");
-  if (sink->set_description(token = tokenstream->get()))
-  {
-    delete token;
-    return 1;
-  }
-  delete token;
 
-  return 0;
+  token.reset(tokenstream->get());
+  sink->set_description(*token);
 }
 
-int tparseParser::parse_rcs_deltatext()
+void tparseParser::parse_rcs_deltatext()
 {
-  rcstoken *revision;
+  auto_ptr<rcstoken> revision, log, text;
-  rcstoken *log;
+
-  rcstoken *text;
   while (1)
   {
-    revision = tokenstream->get();
+    revision.reset(tokenstream->get());
-    if (revision == NULL)
+    if ((*revision).null_token())
       break;
+
     tokenstream->match("log");
-    log = tokenstream->get();
+    log.reset(tokenstream->get());
+
     tokenstream->match("text");
-    text = tokenstream->get();
+    text.reset(tokenstream->get());
-    if (sink->set_revision_info(revision, log, text))
+
-    {
+    sink->set_revision_info(*revision, *log, *text);
-      delete revision;
-      delete log;
-      delete text;
-      return 1;
-    }
-    delete revision;
-    delete log;
-    delete text;
   }
-  return 0;
+  return;
 }

tparse/tparse.h

@@ -30,15 +30,18 @@
  
    Version: $Id$
  */
-#define CHUNK_SIZE 30000
+
 #ifndef __PARSE_H
 #define __PARSE_H
-#include <iostream.h>
+#include <memory>     /* for auto_ptr */
-#include <stdio.h>
+#include <algorithm>  /* for iterator */
-#include <fstream.h>
+#include <exception>  /* for exception */
-#include <string.h>
+#include <iostream>   /* for istream */
-#include <stdlib.h>
+#include <list>       /* for list<> */
+#include <string>     /* for string */
 
+
+#define CHUNK_SIZE 30000
 #define DEFAULT_TOKEN_SIZE 512
 #define DEFAULT_TOKEN_DELTA 10240
 
@@ -46,15 +49,17 @@ using namespace std;
 
 /* This class represents a exception that occured during the parsing
    of a file */
-class RCSParseError
+
+class RCSParseError : public exception
 {
   public:
-    const char *value;
+    string value;
     RCSParseError() {};
     RCSParseError(const char *myvalue)
     {
       value = myvalue;
     };
+    virtual ~RCSParseError() throw() {};
 };
 
 class RCSIllegalCharacter : public RCSParseError
@@ -64,21 +69,27 @@ class RCSIllegalCharacter : public RCSParseError
     {
       value = myvalue;
     };
+    virtual ~RCSIllegalCharacter() throw() {};
 };
 
 class RCSExpected : public RCSParseError
 {
   public:
-    const char *got;
+    string got;
-    const char *wanted;
+    string wanted;
     RCSExpected(const char *mygot, const char *mywanted)
     {
       got = mygot;
       wanted = mywanted;
-    }
+    };
+    RCSExpected(const char *mygot, const char c)
+    {
+      got = mygot;
+      wanted = c;
+    };
+    virtual ~RCSExpected() throw() {};
 };
 
-
 class rcstoken
 {
  public:
@@ -168,26 +179,10 @@ class rcstoken
   rcstoken *copy_begin_len(size_t begin, size_t len);
 };
 
-/* This class is used to store a list of the branches of a revision */
+typedef list<rcstoken> tokenlist;
-class Branche
+typedef tokenlist::iterator tokenlist_iter;
-{
+
-  public:
+
-    rcstoken *name;
-    Branche *next;
-    Branche(rcstoken *myname, Branche *mynext)
-    {
-      name = myname;
-      next = mynext;
-    };
-    ~Branche()
-    {
-      delete name;
-      name = NULL;
-      if (next != NULL)
-        delete next;
-      next = NULL;
-    };
-};
 
 /* This class is a handler that receive the event generated by the parser
    i.e.: When we reach the head revision tag, etc... */
@@ -195,19 +190,19 @@ class Sink
 {
   public:
     Sink() {};
-    virtual int set_head_revision(rcstoken *revision) = 0;
+    virtual ~Sink() throw () {};
-    virtual int set_principal_branch(rcstoken *branch_name) = 0;
+    virtual void set_head_revision(rcstoken &revision) = 0;
-    virtual int define_tag(rcstoken *name, rcstoken *revision) = 0;
+    virtual void set_principal_branch(rcstoken &branch_name) = 0;
-    virtual int set_comment(rcstoken *comment) = 0;
+    virtual void define_tag(rcstoken &name, rcstoken &revision) = 0;
-    virtual int set_description(rcstoken *description) = 0;
+    virtual void set_comment(rcstoken &comment) = 0;
-    virtual int define_revision(rcstoken *revision, long timestamp, 
+    virtual void set_description(rcstoken &description) = 0;
-                                rcstoken *author, rcstoken *state, 
+    virtual void define_revision(rcstoken &revision, long timestamp,
-                                Branche *branches, rcstoken *next) = 0;
+                                 rcstoken &author, rcstoken &state,
-    virtual int set_revision_info(rcstoken *revision,
+                                 tokenlist &branches, rcstoken &next) = 0;
-                                  rcstoken *log,
+    virtual void set_revision_info(rcstoken &revision,
-                                  rcstoken *text) = 0;
+                                   rcstoken &log, rcstoken &text) = 0;
-    virtual int tree_completed() = 0;
+    virtual void tree_completed() = 0;
-    virtual int parse_completed() = 0;
+    virtual void parse_completed() = 0;
 };
 
 /* The class is used to get one by one every token in the file. */
@@ -220,33 +215,30 @@ class TokenParser
     int idx;
     rcstoken *backget;
   public:
-    char *semicol;
     rcstoken *get();
     void unget(rcstoken *token);
     int eof()
     {
       return (input->gcount() == 0);
     };
-    void matchsemicol()
-    {
-      rcstoken *ptr = get();
-      if ((*ptr) != ';')
-        throw RCSExpected(ptr->data, semicol);
-      delete ptr;
-    };
     void match(const char *token)
     {
-      rcstoken *ptr = get();
+      auto_ptr<rcstoken> ptr(get());
       if (*ptr != token)
         throw RCSExpected(ptr->data, token);
-      delete ptr;
+    }
+    void match(const char c)
+    {
+      auto_ptr<rcstoken> token(get());
+
+      if ((*token) != c)
+        throw RCSExpected(token->data, c);
     };
     TokenParser(istream *myinput)
     {
       input = myinput;
       backget = NULL;
       idx = 0;
-      semicol = ";";
       input->read(buf, CHUNK_SIZE);
       if ( (buflength = input->gcount()) == 0 )
         throw RCSParseError("Non-existing file or empty file");
@@ -273,10 +265,10 @@ class tparseParser
   private:
     TokenParser *tokenstream;
     Sink *sink;
-    int parse_rcs_admin();
+    void parse_rcs_admin();
-    int parse_rcs_tree();
+    void parse_rcs_tree();
-    int parse_rcs_description();
+    void parse_rcs_description();
-    int parse_rcs_deltatext();
+    void parse_rcs_deltatext();
   public:
     tparseParser(istream *myinput, Sink* mysink)
     {
@@ -285,25 +277,18 @@ class tparseParser
     }
     void parse()
     {
-      if (parse_rcs_admin())
+      parse_rcs_admin();
-        return;
+      parse_rcs_tree();
-      if (parse_rcs_tree())
-        return;
 
       // many sinks want to know when the tree has been completed so they can
       // do some work to prepare for the arrival of the deltatext
-      if (sink->tree_completed())
+      sink->tree_completed();
-        return;
-
-      if (parse_rcs_description())
-        return;
-      if (parse_rcs_deltatext())
-        return;
 
+      parse_rcs_description();
+      parse_rcs_deltatext();
       // easiest for us to tell the sink it is done, rather than worry about
       // higher level software doing it.
-      if (sink->parse_completed())
+      sink->parse_completed();
-        return;
     }
     ~tparseParser()
     {

tparse/tparsemodule.cpp

@@ -30,7 +30,9 @@
  
    Version: $Id$
 */
-#include <Python.h>
+
+#include <fstream>
+
 #include "tparsemodule.h"
 #include "tparse.cpp"
 
@@ -49,6 +51,49 @@ class PythonException
     PythonException() {};
 };
 
+
+class pyobject
+{
+  private:
+    PyObject *obj;
+  public:
+    pyobject(PyObject *myobj)
+    {
+      obj = myobj;
+    }
+    ~pyobject()
+    {
+      Py_XDECREF(obj);
+    };
+    PyObject *operator*()
+    {
+      return obj;
+    };
+};
+
+
+class pystring : public pyobject
+{
+public:
+  pystring(const char *s) :
+    pyobject(PyString_FromString(s))
+  {};
+  pystring(rcstoken& t) :
+    pyobject(PyString_FromStringAndSize(t.data, t.length))
+  {};
+};
+
+
+static
+void chkpy(PyObject *obj)
+{
+  Py_XDECREF(obj);
+  if (!obj)
+    throw PythonException();
+};
+
+
+
 static PyMethodDef tparseMethods[] = {
   {"parse", tparse, METH_VARARGS, tparse__doc__},
   {NULL, NULL}        /* Sentinel */
@@ -57,6 +102,9 @@ static PyMethodDef tparseMethods[] = {
 void inittparse()
 {
   PyObject *m, *d, *common, *commondict;
+  pystring ver(__version__),
+    dat(__date__),
+    aut(__author__);
   m = Py_InitModule3("tparse", tparseMethods, __doc__);
 
   common = PyImport_ImportModule("common");
@@ -82,17 +130,11 @@ void inittparse()
 
   d = PyModule_GetDict(m);
 
-  PyDict_SetItemString(d, "__version__", PyString_FromString(__version__));
+  PyDict_SetItemString(d, "__version__", *ver);
-  PyDict_SetItemString(d, "__date__", PyString_FromString(__date__));
+  PyDict_SetItemString(d, "__date__", *dat);
-  PyDict_SetItemString(d, "__author__", PyString_FromString(__author__));
+  PyDict_SetItemString(d, "__author__", *aut);
 }
 
-static
-PyObject *rcstoken_to_pystring(rcstoken *token)
-{
-  return PyString_FromStringAndSize(token->data, token->length);
-};
-
 class PythonSink : public Sink
 {
   public:
@@ -100,166 +142,79 @@ class PythonSink : public Sink
     PythonSink(PyObject *mysink)
     {
       sink = mysink;
+      Py_INCREF(sink);
     };
-    int set_head_revision(rcstoken *revision)
+    virtual ~PythonSink() throw ()
     {
-      PyObject *rv = PyObject_CallMethod(sink, "set_head_revision", "s",
+      Py_DECREF(sink);
-                                         revision->data);
-      if (!rv) {
-         if (PyErr_ExceptionMatches(pyRCSStopParser))
-          return 1;
-        else
-          throw PythonException();
-      }
-      else {
-        Py_DECREF(rv);
-      }
-      return 0;
     };
-    int set_principal_branch(rcstoken *branch_name)
+    virtual void set_head_revision(rcstoken &revision)
     {
-      PyObject *rv = PyObject_CallMethod(sink, "set_principal_branch", "s",
+      chkpy(PyObject_CallMethod(sink, "set_head_revision", "s",
-                                         branch_name->data);
+                                revision.data));
-      if (!rv) {
-        if (PyErr_ExceptionMatches(pyRCSStopParser))
-          return 1;
-        else
-          throw PythonException();
-      }
-      else {
-        Py_DECREF(rv);
-      }
-      return 0;
     };
-    int define_tag(rcstoken *name, rcstoken *revision)
+    virtual void set_principal_branch(rcstoken &branch_name)
     {
-      PyObject *rv = PyObject_CallMethod(sink, "define_tag", "ss",
+      chkpy(PyObject_CallMethod(sink, "set_principal_branch",
-                                         name->data, revision->data);
+                                "s", branch_name.data));
-      if (!rv) {
-        if (PyErr_ExceptionMatches(pyRCSStopParser))
-          return 1;
-        else
-          throw PythonException();
-      }
-      else {
-        Py_DECREF(rv);
-      }
-      return 0;
     };
-    int set_comment(rcstoken *comment)
+    virtual void define_tag(rcstoken &name, rcstoken &revision)
     {
-      PyObject *rv = PyObject_CallMethod(sink, "set_comment", "s",
+      chkpy(PyObject_CallMethod(sink, "define_tag", "ss",
-                                         comment->data);
+                                name.data, revision.data));
-      if (!rv) {
-        if (PyErr_ExceptionMatches(pyRCSStopParser))
-          return 1;
-        else
-          throw PythonException();
-      }
-      else {
-        Py_DECREF(rv);
-      }
-      return 0;
     };
-    int set_description(rcstoken *description)
+    virtual void set_comment(rcstoken &comment)
     {
-      PyObject *rv = PyObject_CallMethod(sink, "set_description", "s",
+      pystring c(comment);
-                                         description->data);
+      chkpy(PyObject_CallMethod(sink, "set_comment", "S", *c));
-      if (!rv) {
-        if (PyErr_ExceptionMatches(pyRCSStopParser))
-          return 1;
-        else
-          throw PythonException();
-      }
-      else {
-        Py_DECREF(rv);
-      }
-      return 0;
     };
-    int define_revision(rcstoken *revision, long timestamp, rcstoken *author,
+    virtual void set_description(rcstoken &description)
-                        rcstoken *state, Branche *branches, rcstoken *next)
     {
-      PyObject *pbranchs = PyList_New(0);
+      pystring d(description);
-      Branche *move = branches;
+      chkpy(PyObject_CallMethod(sink, "set_description", "S", *d));
-      while (move != NULL)
+    };
-      {
+    virtual void define_revision(rcstoken &revision, long timestamp,
-        PyObject *str = rcstoken_to_pystring(move->name);
+                                 rcstoken &author, rcstoken &state,
-        PyList_Append(pbranchs, str );
+                                 tokenlist &branches, rcstoken &next)
-        Py_DECREF(str);
+    {
-        move = move->next;
+      pyobject branchlist(PyList_New(0));
-      }
+      tokenlist_iter branch;
 
-      PyObject *rv = PyObject_CallMethod(sink, "define_revision", "slssOs",
+      for (branch = branches.begin(); branch != branches.end(); branch++)
-                                         revision->data,timestamp,
+        {
-                                         author->data,state->data,pbranchs,
+          pystring str(*branch);
-                                         next ? next->data : NULL);
+          PyList_Append(*branchlist, *str);
-      if (!rv) {
+        }
-        Py_DECREF(pbranchs);
-        if (PyErr_ExceptionMatches(pyRCSStopParser))
-          return 1;
-        else
-          throw PythonException();
-      }
-      else {
-        Py_DECREF(rv);
-      }
-      Py_DECREF(pbranchs);
-      return 0;
-    };
-    int set_revision_info(rcstoken *revision, rcstoken *log, rcstoken *text)
-    {
-      PyObject *txt = rcstoken_to_pystring(text);
-      PyObject *rv = PyObject_CallMethod(sink, "set_revision_info", "ssS",
-                                         revision->data,log->data,txt);
-      Py_DECREF(txt);
-      if (!rv) {
-        if (PyErr_ExceptionMatches(pyRCSStopParser))
-          return 1;
-        else
-          throw PythonException();
-      }
-      else {
-        Py_DECREF(rv);
-      }
-      return 0;
-    };
-    int tree_completed()
-    {
-      PyObject *rv = PyObject_CallMethod(sink, "tree_completed", NULL);
-      if (!rv) {
-        if (PyErr_ExceptionMatches(pyRCSStopParser))
-          return 1;
-        else
-          throw PythonException();
-      }
-      else {
-        Py_DECREF(rv);
-      }
-      return 0;
-    };
-    int parse_completed()
-    {
-      PyObject *rv = PyObject_CallMethod(sink, "parse_completed", NULL);
-      if (!rv) {
-        if (PyErr_ExceptionMatches(pyRCSStopParser))
-          return 1;
-        else
-          throw PythonException();
-      }
-      else {
-        Py_DECREF(rv);
-      }
-      return 0;
 
+      chkpy(PyObject_CallMethod(sink, "define_revision", "slssOs",
+                                revision.data,timestamp,
+                                author.data,state.data,*branchlist,
+                                next.data));
+    };
+    virtual void set_revision_info(rcstoken& revision,
+                                   rcstoken& log, rcstoken& text)
+    {
+      pystring l(log), txt(text);
+      chkpy(PyObject_CallMethod(sink, "set_revision_info", "sSS",
+                                revision.data, *l, *txt));
+    };
+    virtual void tree_completed()
+    {
+      chkpy(PyObject_CallMethod(sink, "tree_completed", NULL));
+    };
+    virtual void parse_completed()
+    {
+      chkpy(PyObject_CallMethod(sink, "parse_completed", NULL));
     };
 };
 
 static PyObject * tparse( PyObject *self, PyObject *args)
 {
   char *filename;
-  istream *input;
+  istream *input = NULL;
   PyObject *file = NULL;
   PyObject *hsink;
+  PyObject *rv = Py_None;
 #ifdef GNUC_STDIO_FILEBUF_AVAILABLE
   auto_ptr<streambuf> rdbuf;
 #endif
@@ -284,6 +239,8 @@ static PyObject * tparse( PyObject *self, PyObject *args)
   else
     return NULL;
 
+
+
   if (!PyObject_IsInstance(hsink, PySink))
   {
     PyErr_SetString(PyExc_TypeError,
@@ -298,31 +255,47 @@ static PyObject * tparse( PyObject *self, PyObject *args)
   }
   catch (RCSExpected e)
   {
-    PyObject *exp = PyInstance_New(pyRCSExpected,
+    const char *got = e.got.c_str();
-                                   Py_BuildValue("(ss)", e.got, e.wanted), 
+    const char *wanted = e.wanted.c_str();
-                                   NULL);
+
-    PyErr_SetObject(pyRCSExpected, exp);
+    pyobject arg(Py_BuildValue("(ss)", got, wanted)),
-    return NULL;
+      exp(PyInstance_New(pyRCSExpected, *arg, NULL));
+    PyErr_SetObject(pyRCSExpected, *exp);
+
+    delete [] got;
+    delete [] wanted;
+    rv = NULL;
   }
   catch (RCSIllegalCharacter e)
   {
-    PyObject *exp = PyInstance_New(pyRCSIllegalCharacter,
+    const char *value = e.value.c_str();
-                                   Py_BuildValue("(s)", e.value), NULL);
+
-    PyErr_SetObject(pyRCSIllegalCharacter, exp);
+    pyobject arg(Py_BuildValue("(s)", value)),
-    return NULL;
+      exp(PyInstance_New(pyRCSIllegalCharacter,*arg, NULL));
+    PyErr_SetObject(pyRCSIllegalCharacter, *exp);
+
+    delete [] value;
+    rv = NULL;
   }
   catch (RCSParseError e)
   {
-    PyObject *exp = PyInstance_New(pyRCSParseError,
+    const char *value = e.value.c_str();
-                                   Py_BuildValue("(s)", e.value), NULL);
+
-    PyErr_SetObject(pyRCSParseError, exp);
+    pyobject arg(Py_BuildValue("(s)", value)),
-    return NULL;
+      exp(PyInstance_New(pyRCSParseError, *arg, NULL));
+    PyErr_SetObject(pyRCSParseError, *exp);
+
+    delete [] value;
+    rv = NULL;
   }
   catch (PythonException e)
   {
-    return NULL;
+    if (! PyErr_ExceptionMatches(pyRCSStopParser))
+      rv = NULL;
+    else
+      PyErr_Clear();
   }
 
-  Py_INCREF(Py_None);
+  Py_XINCREF(rv);
-  return Py_None;
+  return rv;
 };

tparse/tparsemodule.h

@@ -29,6 +29,8 @@ extern "C"
 {
 #endif
 
+#include <Python.h>
+
   static char *__doc__ = \
     "This python extension module is a binding to the tparse library.\n" \
     "tparse is a C++ library that offers an API to a performance-oriented\n" \