Fixed a possible race condition with CONN vs. LSTT

3 files changed, 39 insertions, 29 deletions

diff --git a/svdrp.c b/svdrp.c
index 32f9f882..043ffafc 100644
--- a/svdrp.c
+++ b/svdrp.c
@@ -10,7 +10,7 @@
  * and interact with the Video Disk Recorder - or write a full featured
  * graphical interface that sits on top of an SVDRP connection.
  *
- * $Id: svdrp.c 4.33 2018/03/04 14:15:07 kls Exp $
+ * $Id: svdrp.c 4.34 2018/03/17 10:06:54 kls Exp $
  */
 
 #include "svdrp.h"
@@ -47,6 +47,13 @@ static bool DumpSVDRPDataTransfer = false;
 static int SVDRPTcpPort = 0;
 static int SVDRPUdpPort = 0;
 
+enum eSvdrpFetchFlags {
+  sffNone   = 0b00000000,
+  sffConn   = 0b00000001,
+  sffPing   = 0b00000010,
+  sffTimers = 0b00000100,
+  };
+
 // --- cIpAddress ------------------------------------------------------------
 
 class cIpAddress {
@@ -327,8 +334,8 @@ public:
   bool Process(cStringList *Response = NULL);
   bool Execute(const char *Command, cStringList *Response = NULL);
   bool Connected(void) const { return connected; }
-  void SetFetchFlag(eSvdrpFetchFlags Flag);
-  bool HasFetchFlag(eSvdrpFetchFlags Flag);
+  void SetFetchFlag(int Flag);
+  bool HasFetchFlag(int Flag);
   bool GetRemoteTimers(cStringList &Response);
   };
 
@@ -415,8 +422,7 @@ bool cSVDRPClient::Process(cStringList *Response)
                                           serverName = n;
                                           dsyslog("SVDRP %s < %s remote server name is '%s'", Setup.SVDRPHostName, serverIpAddress.Connection(), *serverName);
                                           }
-                                       Execute(cString::sprintf("CONN name:%s port:%d vdrversion:%d apiversion:%d timeout:%d", Setup.SVDRPHostName, SVDRPTcpPort, VDRVERSNUM, APIVERSNUM, Setup.SVDRPTimeout));
-                                       SetFetchFlag(sffTimers);
+                                       SetFetchFlag(sffConn | sffTimers);
                                        connected = true;
                                        }
                                     }
@@ -456,7 +462,7 @@ bool cSVDRPClient::Process(cStringList *Response)
             break; // we read all or nothing!
          }
      if (pingTime.TimedOut())
-        Execute("PING");
+        SetFetchFlag(sffPing);
      }
   return file.IsOpen();
 }
@@ -468,12 +474,12 @@ bool cSVDRPClient::Execute(const char *Command, cStringList *Response)
   return Send(Command) && Process(Response);
 }
 
-void cSVDRPClient::SetFetchFlag(eSvdrpFetchFlags Flags)
+void cSVDRPClient::SetFetchFlag(int Flags)
 {
   fetchFlags |= Flags;
 }
 
-bool cSVDRPClient::HasFetchFlag(eSvdrpFetchFlags Flag)
+bool cSVDRPClient::HasFetchFlag(int Flag)
 {
   bool Result = (fetchFlags & Flag);
   fetchFlags &= ~Flag;
@@ -588,7 +594,7 @@ public:
   void Unlock(void) { mutex.Unlock(); }
   void AddClient(cSVDRPServerParams &ServerParams, const char *IpAddress);
   bool Execute(const char *ServerName, const char *Command, cStringList *Response = NULL);
-  bool GetServerNames(cStringList *ServerNames, eSvdrpFetchFlags FetchFlags = sffNone);
+  bool GetServerNames(cStringList *ServerNames);
   bool TriggerFetchingTimers(const char *ServerName);
   };
 
@@ -635,6 +641,10 @@ void cSVDRPClientHandler::ProcessConnections(void)
   for (int i = 0; i < clientConnections.Size(); i++) {
       cSVDRPClient *Client = clientConnections[i];
       if (Client->Process()) {
+         if (Client->HasFetchFlag(sffConn))
+            Client->Execute(cString::sprintf("CONN name:%s port:%d vdrversion:%d apiversion:%d timeout:%d", Setup.SVDRPHostName, SVDRPTcpPort, VDRVERSNUM, APIVERSNUM, Setup.SVDRPTimeout));
+         if (Client->HasFetchFlag(sffPing))
+            Client->Execute("PING");
          if (Client->HasFetchFlag(sffTimers)) {
             cStringList RemoteTimers;
             if (Client->GetRemoteTimers(RemoteTimers)) {
@@ -712,16 +722,14 @@ bool cSVDRPClientHandler::Execute(const char *ServerName, const char *Command, c
   return false;
 }
 
-bool cSVDRPClientHandler::GetServerNames(cStringList *ServerNames, eSvdrpFetchFlags FetchFlag)
+bool cSVDRPClientHandler::GetServerNames(cStringList *ServerNames)
 {
   cMutexLock MutexLock(&mutex);
   ServerNames->Clear();
   for (int i = 0; i < clientConnections.Size(); i++) {
       cSVDRPClient *Client = clientConnections[i];
-      if (Client->Connected()) {
-         if (FetchFlag == sffNone || Client->HasFetchFlag(FetchFlag))
-            ServerNames->Append(strdup(Client->ServerName()));
-         }
+      if (Client->Connected())
+         ServerNames->Append(strdup(Client->ServerName()));
       }
   return ServerNames->Size() > 0;
 }
@@ -2744,13 +2752,13 @@ void StopSVDRPHandler(void)
   SVDRPServerHandler = NULL;
 }
 
-bool GetSVDRPServerNames(cStringList *ServerNames, eSvdrpFetchFlags FetchFlag)
+bool GetSVDRPServerNames(cStringList *ServerNames)
 {
   bool Result = false;
   cMutexLock MutexLock(&SVDRPHandlerMutex);
   if (SVDRPClientHandler) {
      SVDRPClientHandler->Lock();
-     Result = SVDRPClientHandler->GetServerNames(ServerNames, FetchFlag);
+     Result = SVDRPClientHandler->GetServerNames(ServerNames);
      SVDRPClientHandler->Unlock();
      }
   return Result;
diff --git a/svdrp.h b/svdrp.h
index 167209c4..5330b9aa 100644
--- a/svdrp.h
+++ b/svdrp.h
@@ -4,7 +4,7 @@
  * See the main source file 'vdr.c' for copyright information and
  * how to reach the author.
  *
- * $Id: svdrp.h 4.10 2018/03/04 12:26:17 kls Exp $
+ * $Id: svdrp.h 4.11 2018/03/15 16:30:29 kls Exp $
  */
 
 #ifndef __SVDRP_H
@@ -19,11 +19,6 @@ enum eSvdrpPeerModes {
   spmOnly = 2,
   };
 
-enum eSvdrpFetchFlags {
-  sffNone   = 0b0000,
-  sffTimers = 0b0001,
-  };
-
 extern cStateKey StateKeySVDRPRemoteTimersPoll;
      ///< Controls whether a change to the local list of timers needs to result in
      ///< sending a POLL to the remote clients.
@@ -32,13 +27,10 @@ void SetSVDRPPorts(int TcpPort, int UdpPort);
 void SetSVDRPGrabImageDir(const char *GrabImageDir);
 void StartSVDRPHandler(void);
 void StopSVDRPHandler(void);
-bool GetSVDRPServerNames(cStringList *ServerNames, eSvdrpFetchFlags FetchFlag = sffNone);
+bool GetSVDRPServerNames(cStringList *ServerNames);
      ///< Gets a list of all available VDRs this VDR is connected to via SVDRP,
      ///< and stores it in the given ServerNames list. The list is cleared
      ///< before getting the server names.
-     ///< If FetchFlag is given, only the server names for which the local
-     ///< client has this flag set will be returned, and the client's flag
-     ///< will be cleared.
      ///< Returns true if the resulting list is not empty.
 bool ExecSVDRPCommand(const char *ServerName, const char *Command, cStringList *Response = NULL);
      ///< Sends the given SVDRP Command string to the remote VDR identified
diff --git a/timers.c b/timers.c
index 910ddb7a..8119b858 100644
--- a/timers.c
+++ b/timers.c
@@ -4,7 +4,7 @@
  * See the main source file 'vdr.c' for copyright information and
  * how to reach the author.
  *
- * $Id: timers.c 4.17 2018/03/11 13:03:23 kls Exp $
+ * $Id: timers.c 4.18 2018/03/17 10:07:19 kls Exp $
  */
 
 #include "timers.h"
@@ -952,8 +952,14 @@ bool cTimers::StoreRemoteTimers(const char *ServerName, const cStringList *Remot
          else // processed all right entries
             DelTimer = nl;
          }
-      else if (ir < sr) // still have right entries
+      else if (ir < sr) { // still have right entries
          AddTimer = atoi((*RemoteTimers)[ir]);
+         if (!AddTimer) {
+            esyslog("ERROR: %s: error in timer settings: %s", ServerName, (*RemoteTimers)[ir]);
+            ir++;
+            continue; // let's see if we can process the rest
+            }
+         }
       else // processed all left and right entries
          break;
       if (AddTimer && DelTimer) {
@@ -971,7 +977,7 @@ bool cTimers::StoreRemoteTimers(const char *ServerName, const cStringList *Remot
                   Result = true;
                   }
                else
-                  esyslog("ERROR: %s: error in timer settings: %s", ServerName, v);
+                  esyslog("ERROR: %d@%s: error in timer settings: %s", DelTimer, ServerName, v);
                }
             }
          else // identical timer, nothing to do
@@ -1003,6 +1009,10 @@ bool cTimers::StoreRemoteTimers(const char *ServerName, const cStringList *Remot
             }
          il++;
          }
+      else {
+         esyslog("ERROR: oops while storing remote timers!");
+         break; // let's not get stuck here!
+         }
       }
   return Result;
 }