Don't accidentally reuse stringstreams

Do commandline arguments and paths using std::strings
Make --debug take a parameter for debug event logging level

TCPClient.cpp

@@ -269,8 +269,6 @@ void TCPClient::addChar(char ch, char* response)
     static int mode=MODENORMAL;
     static int charCount=0; // Used to accumulate Newfor
 
-    std::stringstream ss;
-
     response[0]=0;
     switch (mode)
     {
@@ -320,6 +318,7 @@ void TCPClient::addChar(char ch, char* response)
             else
             {
                 // Got a complete non-newfor command
+                std::stringstream ss;
                 ss << "[TCPClient::addChar] finished cmd='" << strlen(_cmd) << "\n";
                 std::cerr << ss.str();
                 if (strlen(_cmd))  // Avoid this being called twice by \n\r combinations

carousel.cpp

@@ -30,13 +30,12 @@ TTXPageStream* Carousel::nextCarousel()
     TTXPageStream* p;
     if (_carouselList.size()==0) return NULL;
 
-    std::stringstream ss;
-
     for (std::list<TTXPageStream*>::iterator it=_carouselList.begin();it!=_carouselList.end();++it)
     {
         p=*it;
         if (p->GetStatusFlag()==TTXPageStream::MARKED && p->GetCarouselFlag()) // only remove it once
         {
+            std::stringstream ss;
             ss << "[Carousel::nextCarousel] Deleted " << p->GetSourcePage() << "\n";
             std::cerr << ss.str();
 
@@ -47,6 +46,7 @@ TTXPageStream* Carousel::nextCarousel()
         }
         else if ((!(p->IsCarousel())) || (p->Special()))
         {
+            std::stringstream ss;
             ss << "[Carousel::nextCarousel] no longer a carousel " << std::hex << p->GetPageNumber() << "\n";
             std::cerr << ss.str();
 

configure.cpp

@@ -4,11 +4,11 @@
 
 using namespace ttx;
 
-int Configure::DirExists(char *path)
+int Configure::DirExists(std::string *path)
 {
     struct stat info;
 
-    if(stat(path, &info ) != 0)
+    if(stat(path->c_str(), &info ) != 0)
         return 0;
     else if(info.st_mode & S_IFDIR)
         return 1;
@@ -26,12 +26,12 @@ Configure::Configure(int argc, char** argv) :
     _serviceStatusString(20, ' '),
     _subtitleRepeats(1)
 {
-    std::stringstream ss;
-    strncpy(_configFile,CONFIGFILE,MAXPATH-1);
-#ifdef _WIN32
-    strncpy(_pageDir,"./pages",MAXPATH-1); // a relative path as a sensible default
+    _configFile = CONFIGFILE;
+
+#ifdef RASPBIAN
+    _pageDir = "/home/pi/teletext";
 #else
-    strcpy(_pageDir,"/home/pi/teletext");
+    _pageDir = "./pages"; // a relative path as a sensible default
 #endif
     // This is where the default header template is defined.
     _headerTemplate = "VBIT2    %%# %%a %d %%b" "\x03" "%H:%M:%S";
@@ -41,7 +41,7 @@ Configure::Configure(int argc, char** argv) :
     _commandPortEnabled = false;
 
     _reverseBits = false;
-    _debug = false;
+    _debugLevel = 0;
 
     _rowAdaptive = false;
     _linesPerField = 16; // default to 16 lines per field
@@ -56,34 +56,63 @@ Configure::Configure(int argc, char** argv) :
     //Scan the command line for overriding the pages file.
     if (argc>1)
     {
-        for (int i=1;i<argc;i++)
+        for (int i=1;i<argc;++i)
         {
-            if (strncmp(argv[i],"--dir",5)==0)
+            std::string arg = argv[i];
+            if (arg == "--dir")
             {
-                i++;
-                strncpy(_pageDir,argv[i],MAXPATH-1);
+                if (i + 1 < argc)
+                    _pageDir = argv[++i];
+                else
+                {
+                    std::cerr << "[Configure::Configure] --dir requires an argument\n";
+                    exit(EXIT_FAILURE);
+                }
             }
-            else if (strncmp(argv[i],"--reverse",9)==0)
+            else if (arg == "--reverse")
             {
                 _reverseBits = true;
             }
-            else if (strncmp(argv[i],"--debug",7)==0)
+            else if (arg == "--debug")
             {
-                _debug = true;
-                std::cerr << "[Configure::Configure] debugging enabled\n";
+                if (i + 1 < argc)
+                {
+                    errno = 0;
+                    char *end_ptr;
+                    const long l = std::strtol(argv[++i], &end_ptr, 10);
+                    if (errno == 0 && *end_ptr == '\0' && l > -1 && l < MAXDEBUGLEVEL)
+                    {
+                        _debugLevel = (int)l;
+
+                        std::stringstream ss;
+                        ss << "[Configure::Configure] debugging enabled at level " << _debugLevel << "\n";
+                        std::cerr << ss.str();
+                    }
+                    else
+                    {
+                        std::cerr << "[Configure::Configure] invalid debug level argument\n";
+                        exit(EXIT_FAILURE);
+                    }
+                }
+                else
+                {
+                    std::cerr << "[Configure::Configure] --debug requires an argument\n";
+                    exit(EXIT_FAILURE);
+                }
             }
         }
     }
 
-    if (!DirExists(_pageDir))
+    if (!DirExists(&_pageDir))
     {
+        std::stringstream ss;
         ss << "[Configure::Configure] " << _pageDir << " does not exist or is not a directory\n";
         std::cerr << ss.str();
         exit(EXIT_FAILURE);
     }
-
-    // TODO: allow overriding config file from command line
 
+    // TODO: allow overriding config file from command line
+    std::stringstream ss;
     ss << "[Configure::Configure] Pages directory is " << _pageDir << "\n";
     ss << "[Configure::Configure] Config file is " << _configFile << "\n";
     std::cerr << ss.str();
@@ -105,15 +134,14 @@ Configure::~Configure()
 int Configure::LoadConfigFile(std::string filename)
 {
     std::ifstream filein(filename.c_str()); // Open the file
-
-    std::stringstream ss;
 
     std::vector<std::string>::iterator iter;
     // these are all the valid strings for config lines
     std::vector<std::string> nameStrings{ "header_template", "initial_teletext_page", "row_adaptive_mode", "network_identification_code", "country_network_identification", "full_field", "status_display", "subtitle_repeats","enable_command_port","command_port","lines_per_field","magazine_priority" };
 
     if (filein.is_open())
     {
+        std::stringstream ss;
         ss << "[Configure::LoadConfigFile] opened " << filename << "\n";
         std::cerr << ss.str();
 
@@ -385,6 +413,7 @@ int Configure::LoadConfigFile(std::string filename)
                 }
                 if (error)
                 {
+                    std::stringstream ss;
                     ss << "[Configure::LoadConfigFile] invalid config line: " << line << "\n";
                     std::cerr << ss.str();
                 }

configure.h

@@ -19,7 +19,8 @@
 #include "ttxline.h"
 
 #define CONFIGFILE "vbit.conf" // default config file name
-#define MAXPATH 132
+
+#define MAXDEBUGLEVEL 3
 
 namespace ttx
 
@@ -33,7 +34,7 @@ class Configure
         Configure(int argc=0, char** argv=NULL);
         ~Configure();
 
-        inline char* GetPageDirectory(){return _pageDir;};
+        inline std::string GetPageDirectory(){return _pageDir;};
 
         std::string GetHeaderTemplate(){return _headerTemplate;}
         bool GetRowAdaptive(){return _rowAdaptive;}
@@ -48,15 +49,15 @@ class Configure
         bool GetCommandPortEnabled(){return _commandPortEnabled;}
         uint16_t GetLinesPerField(){return _linesPerField;}
         bool GetReverseFlag(){return _reverseBits;}
-        bool GetDebugFlag(){return _debug;}
+        int GetDebugLevel(){return _debugLevel;}
         int GetMagazinePriority(uint8_t mag){return _magazinePriority[mag];}
 
         /* these don't really belong in configure, but it's the only place that everything can access */
         void SetMasterClock(time_t t){_masterClock = t;}
         time_t GetMasterClock(){return _masterClock;}
 
     private:
-        int DirExists(char *path);
+        int DirExists(std::string *path);
 
         int LoadConfigFile(std::string filename);
 
@@ -78,12 +79,12 @@ class Configure
         uint16_t _CountryNetworkIdentificationCode;
         std::string _serviceStatusString; /// 20 characters
 
-        char _configFile[MAXPATH]; /// Configuration file name --config
-        char _pageDir[MAXPATH]; /// Configuration file name --dir
+        std::string _configFile; /// Configuration file name --config
+        std::string _pageDir; /// Configuration file name --dir
         uint8_t _subtitleRepeats; /// Number of times a subtitle repeats (typically 1 or 2).
         bool _commandPortEnabled;
         bool _reverseBits;
-        bool _debug;
+        int _debugLevel;
 
         time_t _masterClock;
     };

filemonitor.cpp

@@ -77,8 +77,6 @@ int FileMonitor::readDirectory(std::string path)
     struct dirent *dirp;
     struct stat attrib;
 
-    std::stringstream ss;
-
     DIR *dp;
 
     // Open the directory
@@ -107,6 +105,7 @@ int FileMonitor::readDirectory(std::string path)
             {
                 if (readDirectory(name)) // recurse into directory
                 {
+                    std::stringstream ss;
                     ss << "Error(" << errno << ") recursing into " << name << "\n";
                     std::cerr << ss.str();
                 }
@@ -140,6 +139,7 @@ int FileMonitor::readDirectory(std::string path)
                             // page was not 'special' but now is, add to SpecialPages list
                             q->SetSpecialFlag(true);
                             _pageList->GetMagazines()[mag]->GetSpecialPages()->addPage(q);
+                            std::stringstream ss;
                             ss << "[FileMonitor::run] page was normal, is now special " << std::hex << q->GetPageNumber() << "\n";
                             std::cerr << ss.str();
                             // page will be removed from NormalPages list by the service thread
@@ -150,6 +150,7 @@ int FileMonitor::readDirectory(std::string path)
                             // page was 'special' but now isn't, add to NormalPages list
                             _pageList->GetMagazines()[mag]->GetNormalPages()->addPage(q);
                             q->SetNormalFlag(true);
+                            std::stringstream ss;
                             ss << "[FileMonitor::run] page was special, is now normal " << std::hex << q->GetPageNumber() << "\n";
                             std::cerr << ss.str();
                         }
@@ -160,6 +161,7 @@ int FileMonitor::readDirectory(std::string path)
                             q->SetCarouselFlag(true);
                             q->StepNextSubpage(); // ensure we're pointing at a subpage
                             _pageList->GetMagazines()[mag]->GetCarousel()->addPage(q);
+                            std::stringstream ss;
                             ss << "[FileMonitor::run] page is now a carousel " << std::hex << q->GetPageNumber() << "\n";
                             std::cerr << ss.str();
                         }
@@ -181,6 +183,7 @@ int FileMonitor::readDirectory(std::string path)
             }
             else
             {
+                std::stringstream ss;
                 ss << "[FileMonitor::run] Adding a new page " << dirp->d_name << "\n";
                 std::cerr << ss.str();
                 // A new file. Create the page object and add it to the page list.
@@ -228,12 +231,14 @@ int FileMonitor::readDirectory(std::string path)
                     }
                     else
                     {
+                        std::stringstream ss;
                         ss << "[FileMonitor::run] Failed to add" << dirp->d_name << "\n"; // should never happen
                         std::cerr << ss.str();
                     }
                 }
                 else
                 {
+                    std::stringstream ss;
                     ss << "[FileMonitor::run] Failed to load" << dirp->d_name << "\n";
                     std::cerr << ss.str();
                 }

normalpages.cpp

@@ -23,7 +23,6 @@ void NormalPages::addPage(TTXPageStream* p)
 
 TTXPageStream* NormalPages::NextPage()
 {
-    std::stringstream ss;
     if (_page == nullptr)
     {
         if (_needSorting)
@@ -53,6 +52,7 @@ TTXPageStream* NormalPages::NextPage()
 
         if (_page->GetStatusFlag()==TTXPageStream::MARKED && _page->GetNormalFlag()) // only remove it once
         {
+            std::stringstream ss;
             ss << "[NormalPages::NextPage] Deleted " << _page->GetSourcePage() << "\n";
             std::cerr << ss.str();
             _iter = _NormalPagesList.erase(_iter);
@@ -65,6 +65,7 @@ TTXPageStream* NormalPages::NextPage()
 
         if (_page->Special())
         {
+            std::stringstream ss;
             ss << "[NormalPages::NextPage] page became Special"  << std::hex << _page->GetPageNumber() << "\n";
             std::cerr << ss.str();
             _iter = _NormalPagesList.erase(_iter);

packetDebug.cpp

@@ -52,7 +52,7 @@ bool PacketDebug::IsReady(bool force)
     (void)force; // silence error about unused parameter
     bool result=false;
 
-    if (_configure->GetDebugFlag())
+    if (_configure->GetDebugLevel())
     {
         if (GetEvent(EVENT_FIELD))
         {

pagelist.cpp

@@ -191,13 +191,13 @@ int PageList::Match(char* pageNumber)
         for (std::list<TTXPageStream>::iterator p=_pageList[mag].begin();p!=_pageList[mag].end();++p)
         {
             TTXPageStream* ptr;
-            std::stringstream ss;
             char s[6];
             char* ps=s;
             bool match=true;
             for (ptr=&(*p);ptr!=nullptr;ptr=(TTXPageStream*)ptr->Getm_SubPage()) // For all the subpages in a carousel
             {
                 // Convert the page number into a string so we can compare it
+                std::stringstream ss;
                 ss << std::hex << std::uppercase << std::setw(5) << ptr->GetPageNumber();
                 strcpy(ps,ss.str().c_str());
                 // std::cerr << "[PageList::FindPage] matching " << ps << std::endl;

ttxpage.cpp

@@ -118,7 +118,6 @@ bool TTXPage::m_LoadTTI(std::string filename)
     int lines=0;
     // Open the file
     std::ifstream filein(filename.c_str());
-    std::stringstream ss;
     TTXPage* p=this;
     p->m_Init(); // reset page
     char * ptr;
@@ -263,7 +262,9 @@ bool TTXPage::m_LoadTTI(std::string filename)
                     {
                         std::getline(filein, line);
                         if (line.length()<3)
-                            ss << "invalid page function/coding " << line << "\n";
+                        {
+                            // invalid page function/coding
+                        }
                         else
                         {
                             SetPageFunctionInt(std::strtol(line.substr(0,1).c_str(), &ptr, 16));
@@ -273,15 +274,14 @@ bool TTXPage::m_LoadTTI(std::string filename)
                     }
                     default:
                     {
-                        ss << "Command not understood " << line << "\n";
+                        // line not understood
                     }
                 } // switch
             } // if matched command
             // If the command was not found then skip the rest of the line
         } // seek command
         if (!found) std::getline(filein, line);
     }
-    std::cerr << ss.str();
     filein.close(); // Not sure that we need to close it
     p->Setm_SubPage(nullptr);
     TTXPage::pageChanged=false;
@@ -505,9 +505,9 @@ int TTXPage::GetLanguage()
 
 void TTXPage::SetPageNumber(int page)
 {
-    std::stringstream ss;
     if ((page<0x10000) || (page>0x8ff99))
     {
+        std::stringstream ss;
         ss << "[TTXPage::SetPageNumber] Page number is out of range: " << std::hex << page << std::endl;
         std::cerr << ss.str();
         page = 0x8FF00;