[vdr] Re: Complete all-in-one safe threading patch for vanillavdr-1.3.17 (with poison)

[Rainer Zocholl <UseNet-Posting-Nospam-74308-@zocki.toppoint.de>]

taferner@kde.org(Stefan Taferner)  06.12.04 10:06

>On Monday 06 December 2004 09:55, C.Y.M wrote:
>> With all the confusion and patches flying around these past few
>> days, I have compiled an all-in-one patch that includes all the
>> latest threading fixes for vanilla vdr-1.3.17.  This include the
>> fixes for the race conditions in cThread::Start by Reinhard Nissl
>> and safe threading patches by Rainer Zocholl. Klaus, please consider
>> adding this to vdr-1.3.18.
>>
>> Thanks goes out to everyone that contributed their time and effort
>> on this.  Without them, this patch would not be possible.
>>
>> Thanks,
>> C.Y.M

>Great work!


ACK.

But there are still some "static" traps in "tools.c"
(not likely to trigger, and 2 of them are relative 
difficult to make reentrant as one first has to investigate
what the functions are actually supposed to do ;-).)


>I think it would be helpful to have the poisoning statements
>permanently added to vdr to make plugin writers more aware of the
>threading problems.

Yes.
Or we need a ".configure" that GUARANTIES that a tls-lib is used
and that must be checked at runtime too!




I think it would we usefull to add something into the Makefile
to validate that a g++-3.x compiler used?


reasons why at least a big fat warning should occure:

1) The dependency generator of 2.95 will not include the
   dependency to vdr-header files in "<>", 3.x does.
   "make plugins-clean" costs sometimes a ot time.

2) (?) AFAIK the "#pragma poison" is only supported with 3.x or?(?)

3) Klaus is testing only with a 3.x compiler.



------------------------------------

An (uncomplented!) experiment with tools.c :
There are other files to be changed too which 
uses this funtions.
This patch is (currently) "benign" but will not solve any problems
as long as the "_r" are not called.
Some functions are not ported yet, as it's does
not look so easy and they seem to be used only once. (But
that they are found only once in other code does not mean that
they can be used by only one thread...of course.)



# diff -Naur ../vdr-1.3.17.vanilla/tools.c  tools.c
--- ../vdr-1.3.17.vanilla/tools.c       Sun Nov 21 15:36:34 2004
+++ tools.c     Mon Dec  6 17:05:39 2004
@@ -65,7 +65,7 @@
   safe_write(filedes, &c, sizeof(c));
 }

-char *readline(FILE *f)
+char *readline(FILE *f) // use readline_r instead!
 {
   static char buffer[MAXPARSEBUFFER];
   if (fgets(buffer, sizeof(buffer), f) > 0) {
@@ -77,6 +77,20 @@
   return NULL;
 }

+char *readline_r(FILE *f, char * buffer, size_t size )
+{
+  //static char buffer[MAXPARSEBUFFER];
+  if (fgets(buffer, size, f) > 0) {
+     int l = strlen(buffer) - 1;
+     if (l >= 0 && buffer[l] == '\n')
+        buffer[l] = 0;
+     return buffer;
+     }
+  return NULL;
+}
+
+
+
 char *strcpyrealloc(char *dest, const char *src)
 {
   if (src) {
@@ -167,7 +181,7 @@
   return s;
 }

-const char *strescape(const char *s, const char *chars)
+const char *strescape(const char *s, const char *chars)// NOT THREAD SAFE, DON'T USE THIS!
 {
   static char *buffer = NULL;
   const char *p = s;
@@ -222,7 +236,7 @@
   return strlen(buf);
 }

-int time_ms(void)
+int time_ms(void) // thread safe when called from init() the first time.
 {
   static time_t t0 = 0;
   struct timeval t;
@@ -253,14 +267,22 @@
   return true;
 }

-const char *itoa(int n)
+const char *itoa(int n)// USE itoa_r
 {
   static char buf[16];
   snprintf(buf, sizeof(buf), "%d", n);
   return buf;
 }

-const char *AddDirectory(const char *DirName, const char *FileName)
+
+char *itoa_r(int n, char * buf, size_t size)
+{
+  snprintf(buf, size, "%d", n);
+  return buf;
+}
+
+
+const char *AddDirectory(const char *DirName, const char *FileName) // DONT'T USE IN THREADS
 {
   static char *buf = NULL;
   free(buf);
@@ -449,7 +471,7 @@
   return TargetName ? strdup(TargetName) : NULL;
 }

-bool SpinUpDisk(const char *FileName)
+bool SpinUpDisk(const char *FileName) // DON'T USE IN MULTITHREADING
 {
   static char *buf = NULL;
   for (int n = 0; n < 10; n++) {
@@ -489,7 +511,7 @@
   return 0;
 }

-const char *WeekDayName(int WeekDay)
+const char *WeekDayName(int WeekDay) // DON'T USE, NOT THREAD SAFE
 {
   static char buffer[4];
   WeekDay = WeekDay == 0 ? 6 : WeekDay - 1; // we start with monday==0!
@@ -503,20 +525,21 @@
      return "???";
 }

-const char *WeekDayName(time_t t)
+const char *WeekDayName(time_t t) // DON'T USE, NOT THEAD SAFE
 {
   struct tm tm_r;
   return WeekDayName(localtime_r(&t, &tm_r)->tm_wday);
 }

-const char *DayDateTime(time_t t)
+const char *DayDateTime(time_t t) // DON'T USE, NOT THEAD SAFE
 {
   static char buffer[32];
   if (t == 0)
      time(&t);
   struct tm tm_r;
   tm *tm = localtime_r(&t, &tm_r);
-  snprintf(buffer, sizeof(buffer), "%s %2d.%02d %02d:%02d", WeekDayName(tm->tm_wday), tm->tm_mday, tm->tm_mon + 1, tm
->tm_hour, tm->tm_min);
+  snprintf(buffer, sizeof(buffer), "%s %2d.%02d %02d:%02d"
+         , WeekDayName(tm->tm_wday), tm->tm_mday, tm->tm_mon + 1, tm->tm_hour, tm->tm_min);
   return buffer;
 }


# diff -Naur ../vdr-1.3.17.vanilla/tools.h  tools.h
--- ../vdr-1.3.17.vanilla/tools.h       Sun Oct 31 17:16:37 2004
+++ tools.h     Sun Dec  5 16:42:57 2004
@@ -59,6 +59,7 @@
 ssize_t safe_write(int filedes, const void *buffer, size_t size);
 void writechar(int filedes, char c);
 char *readline(FILE *f);
+char *readline_r(FILE *f, char * buffer, size_t size );
 char *strcpyrealloc(char *dest, const char *src);
 char *strn0cpy(char *dest, const char *src, size_t n);
 char *strreplace(char *s, char c1, char c2);
@@ -75,6 +76,7 @@
 void delay_ms(int ms);
 bool isnumber(const char *s);
 const char *itoa(int n); ///< \warning returns a statically allocated string!
+char *itoa_r(int n, char * buffer, size_t);
 const char *AddDirectory(const char *DirName, const char *FileName); ///< \warning returns a statically allocated string!
 int FreeDiskSpaceMB(const char *Directory, int *UsedMB = NULL);
 bool DirectoryOk(const char *DirName, bool LogErrors = false);