Fixed a possible race condition in cDiseqc::Execute()

5 files changed, 35 insertions, 26 deletions

diff --git a/CONTRIBUTORS b/CONTRIBUTORS
index d7011f00..bc5b0b47 100644
--- a/CONTRIBUTORS
+++ b/CONTRIBUTORS
@@ -2707,3 +2707,6 @@ Dirk Leber <dirk.leber@reel-multimedia.com>
  same as the existing one
  for reporting that TsGetPayload() gets called without checking whether there actually
  is a payload in the given TS packet
+
+Marco Göbenich <mg@needful.de>
+ for reporting a problem with executing diseqc commands from different threads
diff --git a/HISTORY b/HISTORY
index 74f021d1..3dbbb38c 100644
--- a/HISTORY
+++ b/HISTORY
@@ -6607,7 +6607,7 @@ Video Disk Recorder Revision History
 - Avoiding an unecessary call to Recordings.ResetResume() (thanks to Reinhard
   Nissl).
 
-2011-05-21: Version 1.7.19
+2011-05-22: Version 1.7.19
 
 - Fixed cString's operator=(const char *String) in case the given string is the
   same as the existing one (thanks to Dirk Leber).
@@ -6621,3 +6621,6 @@ Video Disk Recorder Revision History
 - The primary device is now only avoided for recording if it is an old SD full
   featured card. This is done through the new function cDevice::AvoidRecording().
 - Subtitle PIDs are now also decrypted (thanks to Reinhard Nissl).
+- Fixed a possible race condition in cDiseqc::Execute() (reported by Marco Göbenich).
+  The return value of cDiseqcs::Get() is now const, so plugin authors may need to
+  adjust their code if they use this function.
diff --git a/diseqc.c b/diseqc.c
index 0168362c..b65d82c6 100644
--- a/diseqc.c
+++ b/diseqc.c
@@ -4,7 +4,7 @@
  * See the main source file 'vdr.c' for copyright information and
  * how to reach the author.
  *
- * $Id: diseqc.c 2.3 2011/05/21 22:07:08 kls Exp $
+ * $Id: diseqc.c 2.4 2011/05/22 10:36:12 kls Exp $
  */
 
 #include "diseqc.h"
@@ -59,7 +59,7 @@ bool cDiseqc::Parse(const char *s)
         polarization = char(toupper(polarization));
         if (polarization == 'V' || polarization == 'H' || polarization == 'L' || polarization == 'R') {
            parsing = true;
-           char *CurrentAction = NULL;
+           const char *CurrentAction = NULL;
            while (Execute(&CurrentAction) != daNone)
                  ;
            parsing = false;
@@ -75,7 +75,7 @@ bool cDiseqc::Parse(const char *s)
   return result;
 }
 
-char *cDiseqc::Wait(char *s)
+const char *cDiseqc::Wait(const char *s) const
 {
   char *p = NULL;
   errno = 0;
@@ -89,19 +89,22 @@ char *cDiseqc::Wait(char *s)
   return NULL;
 }
 
-char *cDiseqc::Codes(char *s)
+const char *cDiseqc::Codes(const char *s) const
 {
-  char *e = strchr(s, ']');
+  const char *e = strchr(s, ']');
   if (e) {
-     numCodes = 0;
-     char *t = s;
-     char *p = s;
+     int NumCodes = 0;
+     const char *t = s;
+     char *p;
      while (t < e) {
-           if (numCodes < MaxDiseqcCodes) {
+           if (NumCodes < MaxDiseqcCodes) {
               errno = 0;
               int n = strtol(t, &p, 16);
               if (!errno && p != t && 0 <= n && n <= 255) {
-                 codes[numCodes++] = uchar(n);
+                 if (parsing) {
+                    codes[NumCodes++] = uchar(n);
+                    numCodes = NumCodes;
+                    }
                  t = skipspace(p);
                  }
               else {
@@ -121,7 +124,7 @@ char *cDiseqc::Codes(char *s)
   return NULL;
 }
 
-cDiseqc::eDiseqcActions cDiseqc::Execute(char **CurrentAction)
+cDiseqc::eDiseqcActions cDiseqc::Execute(const char **CurrentAction) const
 {
   if (!*CurrentAction)
      *CurrentAction = commands;
@@ -146,10 +149,10 @@ cDiseqc::eDiseqcActions cDiseqc::Execute(char **CurrentAction)
 
 cDiseqcs Diseqcs;
 
-cDiseqc *cDiseqcs::Get(int Device, int Source, int Frequency, char Polarization)
+const cDiseqc *cDiseqcs::Get(int Device, int Source, int Frequency, char Polarization) const
 {
   int Devices = 0;
-  for (cDiseqc *p = First(); p; p = Next(p)) {
+  for (const cDiseqc *p = First(); p; p = Next(p)) {
       if (p->Devices()) {
          Devices = p->Devices();
          continue;
diff --git a/diseqc.h b/diseqc.h
index 1b9a41d2..413ed4d9 100644
--- a/diseqc.h
+++ b/diseqc.h
@@ -4,7 +4,7 @@
  * See the main source file 'vdr.c' for copyright information and
  * how to reach the author.
  *
- * $Id: diseqc.h 2.1 2010/02/06 15:14:42 kls Exp $
+ * $Id: diseqc.h 2.2 2011/05/22 10:35:38 kls Exp $
  */
 
 #ifndef __DISEQC_H
@@ -33,15 +33,15 @@ private:
   int lof;
   char *commands;
   bool parsing;
-  uchar codes[MaxDiseqcCodes];
-  int numCodes;
-  char *Wait(char *s);
-  char *Codes(char *s);
+  mutable uchar codes[MaxDiseqcCodes];
+  mutable int numCodes;
+  const char *Wait(const char *s) const;
+  const char *Codes(const char *s) const;
 public:
   cDiseqc(void);
   ~cDiseqc();
   bool Parse(const char *s);
-  eDiseqcActions Execute(char **CurrentAction);
+  eDiseqcActions Execute(const char **CurrentAction) const;
       // Parses the DiSEqC commands and returns the appropriate action code
       // with every call. CurrentAction must be the address of a character pointer,
       // which is initialized to NULL. This pointer is used internally while parsing
@@ -55,12 +55,12 @@ public:
   char Polarization(void) const { return polarization; }
   int Lof(void) const { return lof; }
   const char *Commands(void) const { return commands; }
-  uchar *Codes(int &NumCodes) { NumCodes = numCodes; return numCodes ? codes : NULL; }
+  const uchar *Codes(int &NumCodes) const { NumCodes = numCodes; return numCodes ? codes : NULL; }
   };
 
 class cDiseqcs : public cConfig<cDiseqc> {
 public:
-  cDiseqc *Get(int Device, int Source, int Frequency, char Polarization);
+  const cDiseqc *Get(int Device, int Source, int Frequency, char Polarization) const;
   };
 
 extern cDiseqcs Diseqcs;
diff --git a/dvbdevice.c b/dvbdevice.c
index f32b3500..0d882b15 100644
--- a/dvbdevice.c
+++ b/dvbdevice.c
@@ -4,7 +4,7 @@
  * See the main source file 'vdr.c' for copyright information and
  * how to reach the author.
  *
- * $Id: dvbdevice.c 2.38 2010/05/01 09:47:13 kls Exp $
+ * $Id: dvbdevice.c 2.39 2011/05/22 10:34:49 kls Exp $
  */
 
 #include "dvbdevice.h"
@@ -392,11 +392,11 @@ bool cDvbTuner::SetFrontend(void)
   if (frontendType == SYS_DVBS || frontendType == SYS_DVBS2) {
      unsigned int frequency = channel.Frequency();
      if (Setup.DiSEqC) {
-        cDiseqc *diseqc = Diseqcs.Get(device, channel.Source(), channel.Frequency(), dtp.Polarization());
+        const cDiseqc *diseqc = Diseqcs.Get(device, channel.Source(), channel.Frequency(), dtp.Polarization());
         if (diseqc) {
            if (diseqc->Commands() && (!diseqcCommands || strcmp(diseqcCommands, diseqc->Commands()) != 0)) {
               cDiseqc::eDiseqcActions da;
-              for (char *CurrentAction = NULL; (da = diseqc->Execute(&CurrentAction)) != cDiseqc::daNone; ) {
+              for (const char *CurrentAction = NULL; (da = diseqc->Execute(&CurrentAction)) != cDiseqc::daNone; ) {
                   switch (da) {
                     case cDiseqc::daNone:      break;
                     case cDiseqc::daToneOff:   CHECK(ioctl(fd_frontend, FE_SET_TONE, SEC_TONE_OFF)); break;
@@ -407,7 +407,7 @@ bool cDvbTuner::SetFrontend(void)
                     case cDiseqc::daMiniB:     CHECK(ioctl(fd_frontend, FE_DISEQC_SEND_BURST, SEC_MINI_B)); break;
                     case cDiseqc::daCodes: {
                          int n = 0;
-                         uchar *codes = diseqc->Codes(n);
+                         const uchar *codes = diseqc->Codes(n);
                          if (codes) {
                             struct dvb_diseqc_master_cmd cmd;
                             cmd.msg_len = min(n, int(sizeof(cmd.msg)));