From 529d4a871edcd17ba24ae5a15af90e0ec834d9ac Mon Sep 17 00:00:00 2001
From: rokicki <rokicki@gmail.com>
Date: Wed, 9 Aug 2023 11:16:02 -0700
Subject: [PATCH] More efforts on eliminating C code from the C++ and
 supporting appropriate assignment/copy etc.  This will eliminate memory
 leaks; though small, we might as well do things correctly.  This also means
 we can have multiple distinct puzzle definitions and the like active at once.

---
 src/cpp/cmdlineops.cpp |  12 ++---
 src/cpp/puzdef.cpp     |   2 +-
 src/cpp/puzdef.h       | 101 +++++++++++++++++++++++++++++++----------
 src/cpp/readksolve.cpp |  44 +++++++++---------
 src/cpp/readksolve.h   |   2 +-
 src/cpp/rotations.cpp  |   3 +-
 src/cpp/util.cpp       |   9 ----
 src/cpp/util.h         |   1 -
 8 files changed, 107 insertions(+), 67 deletions(-)

diff --git a/src/cpp/cmdlineops.cpp b/src/cpp/cmdlineops.cpp
index 51319c76..c2aec3ae 100644
--- a/src/cpp/cmdlineops.cpp
+++ b/src/cpp/cmdlineops.cpp
@@ -75,12 +75,12 @@ void processscrambles(istream *f, puzdef &pd, prunetable &pt, generatingset *gs)
          break ;
       if (toks[0] == "Scramble" || toks[0] == "ScrambleState" || toks[0] == "StartState") {
          expect(toks, 2) ;
-         scramblename = twstrdup(toks[1].c_str()) ; ;
-         setval p = readposition(pd, 'S', f, checksum, toks[0] == "ScrambleState" || toks[0] == "StartState") ;
+         scramblename = toks[1] ;
+         allocsetval p = readposition(pd, 'S', f, checksum, toks[0] == "ScrambleState" || toks[0] == "StartState") ;
          solveit(pd, pt, scramblename, p, gs) ;
       } else if (toks[0] == "ScrambleAlg") {
          expect(toks, 2) ;
-         scramblename = twstrdup(toks[1].c_str()) ; ;
+         scramblename = toks[1] ;
          pd.assignpos(p1, pd.solved) ;
          while (1) {
             toks = getline(f, checksum) ;
@@ -111,13 +111,13 @@ void readfirstscramble(istream *f, puzdef &pd, setval sv) {
          break ;
       if (toks[0] == "Scramble" || toks[0] == "StartState") {
          expect(toks, 2) ;
-         scramblename = twstrdup(toks[1].c_str()) ; ;
-         setval p = readposition(pd, 'S', f, checksum, toks[0] == "StartState") ;
+         scramblename = toks[1] ;
+         allocsetval p = readposition(pd, 'S', f, checksum, toks[0] == "StartState") ;
          pd.assignpos(sv, p) ;
          return ;
       } else if (toks[0] == "ScrambleAlg") {
          expect(toks, 2) ;
-         scramblename = twstrdup(toks[1].c_str()) ; ;
+         scramblename = toks[1] ;
          pd.assignpos(sv, pd.solved) ;
          while (1) {
             toks = getline(f, checksum) ;
diff --git a/src/cpp/puzdef.cpp b/src/cpp/puzdef.cpp
index 66d18483..d4fd99e1 100644
--- a/src/cpp/puzdef.cpp
+++ b/src/cpp/puzdef.cpp
@@ -118,7 +118,7 @@ ll puzdef::order(const vector<int> cc) {
          r = lcm(r, i) ;
    return r ;
 }
-void puzdef::addillegal(const char *setname, int pos, int val) {
+void puzdef::addillegal(const string &setname, int pos, int val) {
    if (val > 64)
       error("! cannot use illegal on sets with more than 64 elements") ;
    if (val <= 0)
diff --git a/src/cpp/puzdef.h b/src/cpp/puzdef.h
index 816ca9f1..c77c27fa 100644
--- a/src/cpp/puzdef.h
+++ b/src/cpp/puzdef.h
@@ -65,24 +65,52 @@ struct illegal_t {
    int pos ;
    ull mask ;
 } ;
+struct puzdef ;
+//  These cannot be copied or assigned
+struct stacksetval : setval {
+   stacksetval(const puzdef &pd) ;
+   stacksetval(const puzdef &pd, const setval iv) ;
+   stacksetval(const stacksetval &) = delete ;
+   stacksetval(stacksetval &&) = delete ;
+   stacksetval& operator=(const stacksetval &) = delete ;
+   stacksetval& operator=(stacksetval &&) = delete ;
+   ~stacksetval() { delete [] dat ; }
+   const puzdef *owner ;
+} ;
+struct allocsetval : setval {
+   allocsetval() : setval(0), sz(0) {}
+   allocsetval(const puzdef &pd, int) ;
+   allocsetval(const puzdef &pd, const setval &iv) ;
+   allocsetval(const allocsetval &) ;
+   allocsetval(allocsetval &&) ;
+   allocsetval& operator=(const allocsetval &) ;
+   allocsetval& operator=(allocsetval &&) ;
+   ~allocsetval() {
+      if (dat) {
+         delete[] dat ;
+         dat = 0 ;
+      }
+   }
+   int sz ;
+} ;
 struct moove {
-   moove() : name(), pos(0), cost(1) {}
+   moove(const puzdef &pd, const setval &iv) : name(), pos(pd, iv), cost(1) {}
    string name ;
-   setval pos ;
+   allocsetval pos ;
    int cost, base, twist, cs ;
 } ;
 extern int origroup ;
 struct movealias {
-   const char *src, *dst ;
+   string src, dst ;
 } ;
 struct puzdef {
-   puzdef() : name(), setdefs(), solved(0), totsize(0), id(0),
+   puzdef() : name(), setdefs(), solved(), totsize(0), id(),
               logstates(0), llstates(0), checksum(0), haveillegal(0),
               wildo(0), uniq(1)
               {}
    string name ;
    setdefs_t setdefs ;
-   setval solved ;
+   allocsetval solved ;
    vector<moove> basemoves, moves, parsemoves, rotations, expandedrotations, rotgroup ;
    vector<movealias> aliases ;
    vector<movealias> moveseqs ;
@@ -92,7 +120,7 @@ struct puzdef {
    vector<ull> commutes ;
    int totsize ;
    int ncs ;
-   setval id ;
+   allocsetval id ;
    double logstates ;
    unsigned long long llstates ;
    ull checksum ;
@@ -119,6 +147,7 @@ struct puzdef {
    int permwrong(const setval a, const setval b, ull mask=-1) const ;
    vector<int> cyccnts(const setval a, ull sets=-1) const ;
    static ll order(const vector<int> cc) ;
+   vector<allocsetval> stacksetvals ;
    void mul(const setval a, const setval b, setval c) const {
       const uchar *ap = a.dat ;
       const uchar *bp = b.dat ;
@@ -330,27 +359,51 @@ struct puzdef {
       int twist = (o - mv.twist) % o ;
       return mvind-mv.twist+twist ;
    }
-   void addillegal(const char *setname, int pos, int val) ;
+   void addillegal(const string &setname, int pos, int val) ;
    void pow(const setval a, setval b, ll cnt) const ;
    void inv(const setval a, setval b) const ;
 } ;
-struct stacksetval : setval {
-   stacksetval(const puzdef &pd) : setval(new uchar[pd.totsize]) {
-      memcpy(dat, pd.id.dat, pd.totsize) ;
-   }
-   stacksetval(const puzdef &pd, const setval iv) : setval(new uchar[pd.totsize]) {
-      memcpy(dat, iv.dat, pd.totsize) ;
-   }
-   ~stacksetval() { delete [] dat ; }
-} ;
-struct allocsetval : setval {
-   allocsetval(const puzdef &pd, const setval &iv) : setval(new uchar[pd.totsize]) {
-      memcpy(dat, iv.dat, pd.totsize) ;
-   }
-   ~allocsetval() {
-      // we drop memory here; need fix
-   }
-} ;
+inline stacksetval::stacksetval(const puzdef &pd) {
+   dat = new uchar[pd.totsize] ;
+   owner = &pd ;
+   memcpy(dat, pd.id.dat, pd.totsize) ;
+}
+inline stacksetval::stacksetval(const puzdef &pd, const setval iv) {
+   dat = new uchar[pd.totsize] ;
+   owner = &pd ;
+   memcpy(dat, iv.dat, pd.totsize) ;
+}
+inline allocsetval::allocsetval(const puzdef &pd, const setval &iv) {
+   dat = new uchar[pd.totsize] ;
+   sz = pd.totsize ;
+   memcpy(dat, iv.dat, pd.totsize) ;
+}
+inline allocsetval::allocsetval(const puzdef &pd, int) {
+   dat = new uchar[pd.totsize] ;
+   sz = pd.totsize ;
+}
+inline allocsetval::allocsetval(const allocsetval &v) {
+   dat = new uchar[v.sz] ;
+   sz = v.sz ;
+   memcpy(dat, v.dat, sz) ;
+}
+inline allocsetval::allocsetval(allocsetval &&v) {
+   dat = v.dat ;
+   v.dat = 0 ;
+   sz = v.sz ;
+}
+inline allocsetval& allocsetval::operator=(const allocsetval &v) {
+   dat = new uchar[v.sz] ;
+   sz = v.sz ;
+   memcpy(dat, v.dat, sz) ;
+   return *this ;
+}
+inline allocsetval& allocsetval::operator=(allocsetval &&v) {
+   dat = v.dat ;
+   v.dat = 0 ;
+   sz = v.sz ;
+   return *this ;
+}
 extern vector<allocsetval> posns ;
 extern vector<int> movehist ;
 void calculatesizes(puzdef &pd) ;
diff --git a/src/cpp/readksolve.cpp b/src/cpp/readksolve.cpp
index e7f09f3a..118793fb 100644
--- a/src/cpp/readksolve.cpp
+++ b/src/cpp/readksolve.cpp
@@ -121,8 +121,8 @@ int omitset(string s) {
       return 1 ;
    return 0 ;
 }
-setval readposition(puzdef &pz, char typ, istream *f, ull &checksum, bool zero_indexed) {
-   setval r((uchar *)calloc(pz.totsize, 1)) ;
+allocsetval readposition(puzdef &pz, char typ, istream *f, ull &checksum, bool zero_indexed) {
+   allocsetval r(pz, pz.id) ;
    int curset = -1 ;
    int numseq = 0 ;
    int ignore = 0 ;
@@ -307,7 +307,7 @@ puzdef readdef(istream *f) {
             inerror("! Name in wrong place") ;
          state++ ;
          expect(toks, 2) ;
-         pz.name = twstrdup(toks[1].c_str()) ; ;
+         pz.name = toks[1] ;
       } else if (toks[0] == "Set") {
          if (state == 0) {
             pz.name = "Unnamed" ;
@@ -319,7 +319,7 @@ puzdef readdef(istream *f) {
          if (omitset(toks[1]))
             continue ;
          setdef sd ;
-         sd.name = twstrdup(toks[1].c_str()) ;
+         sd.name = toks[1] ;
          sd.size = getnumber(1, toks[2]) ;
          sd.omod = getnumber(1, toks[3]) ;
          if (sd.omod > 127)
@@ -358,13 +358,24 @@ puzdef readdef(istream *f) {
             inerror("! Solved in wrong place") ;
          state++ ;
          expect(toks, 1) ;
+         pz.id = allocsetval(pz, -1) ;
+         uchar *p = pz.id.dat ;
+         for (int i=0; i<(int)pz.setdefs.size(); i++) {
+            int n = pz.setdefs[i].size ;
+            for (int j=0; j<n; j++)
+               p[j] = j ;
+            p += n ;
+            for (int j=0; j<n; j++)
+               p[j] = 0 ;
+            p += n ;
+         }
          pz.solved = readposition(pz, 's', f, checksum, toks[0] == "StartState") ;
       } else if (toks[0] == "Move" || toks[0] == "MoveTransformation") {
          if (state != 2)
             inerror("! Move in wrong place") ;
          expect(toks, 2) ;
-         moove m ;
-         m.name = twstrdup(toks[1].c_str()) ;
+         moove m(pz, pz.id) ;
+         m.name = toks[1] ;
          m.pos = readposition(pz, 'm', f, checksum, toks[0] == "MoveTransformation") ;
          m.cost = 1 ;
          m.twist = 1 ;
@@ -379,8 +390,7 @@ puzdef readdef(istream *f) {
          if (state != 2)
             inerror("! MoveAlias in wrong place") ;
          expect(toks, 3) ;
-         pz.aliases.push_back(
-                      {twstrdup(toks[1].c_str()), twstrdup(toks[2].c_str())}) ;
+         pz.aliases.push_back({toks[1], toks[2]}) ;
       } else if (toks[0] == "MoveSequence") {
          if (state != 2)
             inerror("! MoveSequence in wrong place") ;
@@ -392,8 +402,7 @@ puzdef readdef(istream *f) {
                seq += " " ;
             seq += toks[i] ;
          }
-         pz.moveseqs.push_back(
-                      {twstrdup(toks[1].c_str()), twstrdup(seq.c_str())}) ;
+         pz.moveseqs.push_back({toks[1], seq}) ;
       } else {
          inerror("! unexpected first token on line ", toks[0]) ;
       }
@@ -407,17 +416,6 @@ puzdef readdef(istream *f) {
    if (pz.moves.size() == 0)
       inerror("! puzzle must have moves") ;
    lineno = 0 ;
-   pz.id = setval((uchar *)calloc(pz.totsize, 1)) ;
-   uchar *p = pz.id.dat ;
-   for (int i=0; i<(int)pz.setdefs.size(); i++) {
-      int n = pz.setdefs[i].size ;
-      for (int j=0; j<n; j++)
-         p[j] = j ;
-      p += n ;
-      for (int j=0; j<n; j++)
-         p[j] = 0 ;
-      p += n ;
-   }
    if (distinguishall) {
       pz.solved = pz.id ;
    }
@@ -430,7 +428,7 @@ void expandmoveset(const puzdef &pd, vector<moove> &moves, vector<moove> &newmov
       moove &m = moves[i] ;
       if (quarter && m.cost > 1)
          continue ;
-      vector<setval> movepowers ;
+      vector<allocsetval> movepowers ;
       movepowers.push_back(m.pos) ;
       pd.assignpos(p1, m.pos) ;
       pd.assignpos(p2, m.pos) ;
@@ -458,7 +456,7 @@ void expandmoveset(const puzdef &pd, vector<moove> &moves, vector<moove> &newmov
             if (tw < 0)
                s2 += "'" ;
             newnames.push_back(s2) ;
-            m2.name = twstrdup(s2.c_str()) ;
+            m2.name = s2 ;
          }
          newmoves.push_back(m2) ;
       }
diff --git a/src/cpp/readksolve.h b/src/cpp/readksolve.h
index 97db7c3c..4b83485a 100644
--- a/src/cpp/readksolve.h
+++ b/src/cpp/readksolve.h
@@ -9,7 +9,7 @@
 vector<string> getline(istream *f, ull &checksum) ;
 void expect(const vector<string> &toks, int cnt) ;
 int getnumber(int minval, const string &s) ;
-setval readposition(puzdef &pz, char typ, istream *f, ull &checksum, bool zero_indexed) ;
+allocsetval readposition(puzdef &pz, char typ, istream *f, ull &checksum, bool zero_indexed) ;
 puzdef readdef(istream *f) ;
 void addmovepowers(puzdef &pd) ;
 extern int nocorners, nocenters, noedges, ignoreori, distinguishall ;
diff --git a/src/cpp/rotations.cpp b/src/cpp/rotations.cpp
index 3379d7f0..9e2db5c4 100644
--- a/src/cpp/rotations.cpp
+++ b/src/cpp/rotations.cpp
@@ -64,9 +64,8 @@ void calcrotations(puzdef &pd) {
    vector<moove> &q = pd.rotgroup ;
    set<vector<uchar>> seen ;
    seen.insert(setvaltovec(pd, pd.id)) ;
-   moove m ;
+   moove m(pd, pd.id) ;
    m.name = "(identity)" ;
-   m.pos = allocsetval(pd, pd.id) ;
    m.cost = 0 ;
    m.twist = 0 ;
    q.push_back(m) ;
diff --git a/src/cpp/util.cpp b/src/cpp/util.cpp
index 8c9e52cf..f0469c98 100644
--- a/src/cpp/util.cpp
+++ b/src/cpp/util.cpp
@@ -35,15 +35,6 @@ void error(string msg, string extra) {
 void warn(string msg, string extra) {
    cerr << msg << extra << endl ;
 }
-/*
- *   strdup is going through some issues: POSIX vs C++, so we just
- *   implement it ourselves.
- */
-const char *twstrdup(const char *s) {
-   char *r = (char *)malloc(strlen(s)+1) ;
-   strcpy(r, s) ;
-   return r ;
-}
 static mt19937 *rng ;
 // avoid static initialization fiasco by always seeding.
 void mysrand(int n) {
diff --git a/src/cpp/util.h b/src/cpp/util.h
index f49c715b..053e8cfa 100644
--- a/src/cpp/util.h
+++ b/src/cpp/util.h
@@ -23,7 +23,6 @@ double walltime() ;
 double duration() ;
 void error(string msg, string extra="") ;
 void warn(string msg, string extra="") ;
-const char *twstrdup(const char *s) ;
 double myrand(int n) ;
 void mysrand(int n) ;
 ll gcd(ll a, ll b) ;