Lg_add_pattern_list return bug

Makefile.am

@@ -31,7 +31,6 @@ src_lib_liblightgrepint_la_SOURCES = \
 	include/chain.h \
 	include/codegen.h \
 	include/compiler.h \
-	include/container_out.h \
 	include/decoders/asciidecoder.h \
 	include/decoders/bytesource.h \
 	include/decoders/decoder.h \
@@ -299,7 +298,6 @@ test_test_SOURCES = \
 	test/test_oceencoder.cpp \
 	test/test_options.cpp \
 	test/test_optparser.cpp \
-	test/test_ostream_join_iterator.cpp \
 	test/test_parser.cpp \
 	test/test_parsetree.cpp \
 	test/test_parseutil.cpp \

examples/c_example/main.c

@@ -48,7 +48,7 @@ void searchText(const char** textArray, unsigned int numStrings, LG_HCONTEXT sea
   lg_closeout_search(searcher, (void*)17, getHit);
 }
 
-int main(int, char**) {
+int main(void) {
   const char* keys[] = {"mary", "lamb", "[a-z]+"};
   const unsigned int kcount = sizeof(keys)/sizeof(keys[0]);
 

include/lightgrep/api.h

@@ -119,7 +119,7 @@ extern "C" {
 
   // Create and destory an LG_HPATTERN.
   // This can be reused when parsing pattern strings to avoid re-allocating memory.
-  LG_HPATTERN lg_create_pattern();
+  LG_HPATTERN lg_create_pattern(void);
 
   void lg_destroy_pattern(LG_HPATTERN hPattern);
 
@@ -135,10 +135,8 @@ extern "C" {
   // and fragmentation. A good rule of thumb is to pass the total number of
   // characters in all of the keywords. Everything will work fine with 0,
   // though.
-  LG_HFSM lg_create_fsm(
-    unsigned int patternCountHint,
-    unsigned int numFsmStateSizeHint
-  );
+  LG_HFSM lg_create_fsm(unsigned int patternCountHint,
+                        unsigned int numFsmStateSizeHint);
 
   void lg_destroy_fsm(LG_HFSM hFsm);
 
@@ -149,13 +147,11 @@ extern "C" {
   // are no constraints on the value of userIndex. Set it for each pattern
   // to whatever you want to see as the LG_PatternInfo::UserIndex for that
   // pattern.
-  int lg_add_pattern(
-    LG_HFSM hFsm,
-    LG_HPATTERN hPattern,
-    const char* encoding,
-    uint64_t userIndex,
-    LG_Error** err
-  );
+  int lg_add_pattern(LG_HFSM hFsm,
+                     LG_HPATTERN hPattern,
+                     const char* encoding,
+                     uint64_t userIndex,
+                     LG_Error** err);
 
   // Adds patterns to the FSM and Program. Each line of the pattern string
   // shall be formatted as tab separated columns:
@@ -170,15 +166,13 @@ extern "C" {
   // The values of defaultEncodings and defaultOptions are used in case of
   // omitted columns. The "source" parameter indicates the source of the
   // patterns string (e.g., a path) and is used in error messages.
-  int lg_add_pattern_list(
-    LG_HFSM hFsm,
-    const char* patterns,
-    const char* source,
-    const char** defaultEncodings,
-    unsigned int defaultEncodingsNum,
-    const LG_KeyOptions* defaultOptions,
-    LG_Error** err
-  );
+  int lg_add_pattern_list(LG_HFSM hFsm,
+                          const char* patterns,
+                          const char* source,
+                          const char** defaultEncodings,
+                          unsigned int defaultEncodingsNum,
+                          const LG_KeyOptions* defaultOptions,
+                          LG_Error** err);
 
   // The number of pattern-encoding pairs recognized by the FSM. This
   // will be one greater than the maximum pattern index accepted by
@@ -260,11 +254,11 @@ extern "C" {
   // In particular, it may not be possible to determine the full length of a
   // hit until the entire byte stream has been searched...
   uint64_t lg_search(LG_HCONTEXT hCtx,
-                         const char* bufStart,
-                         const char* bufEnd,   // pointer past the end of the buffer, i.e. bufEnd - bufStart == length of buffer
-                         const uint64_t startOffset,   // Increment this with each call, by the length of the previous buffer. i.e., startOffset += bufEnd - bufStart;
-                         void* userData,       // pass in what you like, it will be passed through to the callback function
-                         LG_HITCALLBACK_FN callbackFn);
+                     const char* bufStart,
+                     const char* bufEnd,   // pointer past the end of the buffer, i.e. bufEnd - bufStart == length of buffer
+                     const uint64_t startOffset,   // Increment this with each call, by the length of the previous buffer. i.e., startOffset += bufEnd - bufStart;
+                     void* userData,       // pass in what you like, it will be passed through to the callback function
+                     LG_HITCALLBACK_FN callbackFn);
 
   // ...which is why it's important you call lg_closeout_search() when finished
   // searching a byte stream. This will flush out any remaining search hits.
@@ -275,11 +269,11 @@ extern "C" {
   // Return value is the least offset for which a hit could still be returned
   // by further searching.
   uint64_t lg_search_resolve(LG_HCONTEXT hCtx,
-                         const char* bufStart,
-                         const char* bufEnd,
-                         const uint64_t startOffset,
-                         void* userData,
-                         LG_HITCALLBACK_FN callbackFn);
+                             const char* bufStart,
+                             const char* bufEnd,
+                             const uint64_t startOffset,
+                             void* userData,
+                             LG_HITCALLBACK_FN callbackFn);
 
 #ifdef __cplusplus
 }

include/options.h

@@ -25,7 +25,7 @@
 #include <string>
 #include <vector>
 
-#include <boost/program_options.hpp>
+#include "boost_program_options.h"
 namespace po = boost::program_options;
 
 class Options {

src/cmd/options.cpp

@@ -21,7 +21,6 @@
 
 #include <iostream>
 
-#include "ostream_join_iterator.h"
 #include "util.h"
 
 std::ostream& Options::openOutput() const {
@@ -54,10 +53,9 @@ std::vector<std::pair<std::string,std::string>> Options::getPatternLines() const
       os << p << '\t';
 
       // encodings
-      std::copy(
-        Encodings.begin(), Encodings.end(),
-        ostream_join_iterator<std::string>(os, ",")
-      );
+      for (const auto& e : Encodings) {
+        os << e << ',';
+      }
 
       os << '\t';
 
@@ -197,9 +195,10 @@ void Options::validateAndPopulateSearchOptions(const po::variables_map& optsMap,
   }
 }
 
-void Options::populateSampleOptions(const po::variables_map& optsMap, std::vector<std::string>& pargs) {
-  SampleLimit =
-        std::numeric_limits<std::set<std::string>::size_type>::max();
+void Options::populateSampleOptions(const po::variables_map&, std::vector<std::string>& pargs) {
+  // FIXME: the map's no longer used and this function needs a rethink with tests.
+  // For example, SampleLimit is set to the max value of size_t, so it's a bad default.
+  SampleLimit = std::numeric_limits<std::set<std::string>::size_type>::max();
   LoopLimit = 1;
 
   if (!pargs.empty()) {
@@ -212,3 +211,4 @@ void Options::populateSampleOptions(const po::variables_map& optsMap, std::vecto
     }
   }
 }
+

src/lib/lightgrep_c_api.cpp

@@ -187,29 +187,36 @@ int lg_add_pattern(LG_HFSM hFsm,
 
 namespace {
   template <class E>
-  void addPatternHelper(LG_HFSM hFsm,
+  int addPatternHelper(LG_HFSM hFsm,
                   LG_HPATTERN hPat,
                   const std::string& pat,
-                  LG_KeyOptions* keyOpts,
+                  const LG_KeyOptions* keyOpts,
                   const E& encodings,
                   int lnum,
                   LG_Error**& err)
   {
-    lg_parse_pattern(hPat, pat.c_str(), keyOpts, err);
+    auto ret = lg_parse_pattern(hPat, pat.c_str(), keyOpts, err);
     if (*err) {
       (*err)->Index = lnum;
       err = &((*err)->Next);
-      return;
+      return -1;
+    }
+    else if (ret == 0) {
+      return -1;
     }
 
+    bool hadError = false;
     for (const std::string& enc : encodings) {
-      lg_add_pattern(hFsm, hPat, enc.c_str(), lnum, err);
+      ret = lg_add_pattern(hFsm, hPat, enc.c_str(), lnum, err);
       if (*err) {
         (*err)->Index = lnum;
         err = &((*err)->Next);
-        continue;
+      }
+      if (ret == -1) {
+        hadError = true;
       }
     }
+    return hadError ? -1 : 0;
   }
 
   int addPatternList(LG_HFSM hFsm,
@@ -220,6 +227,7 @@ namespace {
                      const LG_KeyOptions* defaultOptions,
                      LG_Error** err)
   {
+    bool hadError = false;
     std::unique_ptr<PatternHandle,void(*)(PatternHandle*)> ph(
       lg_create_pattern(),
       lg_destroy_pattern
@@ -248,6 +256,7 @@ namespace {
 
       if (ccur == cend) { // FIXME: is this possible?
         if (err) {
+          hadError = true;
           *err = makeError("no pattern", nullptr, nullptr, source, lnum);
           err = &((*err)->Next);
         }
@@ -257,12 +266,10 @@ namespace {
       // read the pattern
       const std::string pat(*ccur);
 
-      LG_KeyOptions opts(*defaultOptions);
-
-      if (++ccur != cend) {
+      if (++ccur != cend) { // has encodings & maybe options
         // read the encoding list
-        const std::string el(*ccur);
-        const tokenizer etok(el, char_separator(","));
+        const std::string encList(*ccur);
+        const tokenizer etok(encList, char_separator(","));
 
         if (etok.begin() == etok.end()) {
           if (err) {
@@ -272,10 +279,12 @@ namespace {
             );
             err = &((*err)->Next);
           }
+          hadError = true;
           continue;
         }
 
         // read the options
+        LG_KeyOptions opts(*defaultOptions);
         if (++ccur != cend) {
           opts.FixedString = boost::lexical_cast<bool>(*ccur);
           if (++ccur != cend) {
@@ -285,16 +294,17 @@ namespace {
             }
           }
         }
-
-        addPatternHelper(hFsm, ph.get(), pat, &opts, etok, lnum, err);
+        if (addPatternHelper(hFsm, ph.get(), pat, &opts, etok, lnum, err) != 0) {
+          hadError = true;
+        }
       }
-      else {
-        // use default encodings and options
-        addPatternHelper(hFsm, ph.get(), pat, &opts, defEncs, lnum, err);
+      else { // use default encodings and options
+        if (addPatternHelper(hFsm, ph.get(), pat, defaultOptions, defEncs, lnum, err) != 0) {
+          hadError = true;
+        }
       }
     }
-
-    return err ? -1 : 0;
+    return hadError ? -1 : 0;
   }
 }
 

src/lib/lightgrep_c_util.cpp

@@ -15,11 +15,6 @@
  *
  */
 
-/*
-#include "container_out.h"
-#include <iostream>
-*/
-
 #include <algorithm>
 #include <cctype>
 #include <limits>