[vdr] vdr 1.3.25 thread problems

[Darren Salt]

I demand that Rainer Zocholl may or may not have written...

> Darren Salt  01.06.05 20:28
>> I demand that Rainer Zocholl may or may not have written...
>>> Again:
>>> The most beloved problem: Reentrancy...

>>> This C-Function in tools.c looks suspicious:

>>> cString TimeToString(time_t t)
>>> {
>>>   char buffer[32];
>>>   if (ctime_r(&t, buffer)) {
>>>      buffer[strlen(buffer) - 1] = 0; // strip trailing newline
>>>      return buffer;
>>>      }
>>>   return "???";
>>> }

>> That's harmless.

> On the first view it does not look so ;-)

True. That's one of the features of C++ ;-)

>> Those return statements are effectively 'return cString (<string>,
>> false)', and the cString constructor will call strdup() if its second
>> parameter is false (and note that that parameter is declared as having a
>> default value).

> Ah, ok, but's not very effective to copy each string several times IMHO.

You have something which is guaranteed to be freeable by the destructor...

> Pardon my stupid question:
> And who is freeing that malloced memory later?

The caller, as soon as the object goes out of scope (the compiler will
automatically insert a call to the object's destructor at that point).

> for example:

> cTDT::cTDT(const u_char *Data)
> :SI::TDT(Data, false)
> {
[snip]
>      isyslog("System Time = %s (%ld)\n", *TimeToString(loctim),loctim);
>      isyslog("Local Time  = %s (%ld)\n", *TimeToString(sattim),sattim);
[snip; no obvious free or delete]
> }

> The pointer is not stored anywhere.
> (At least not obviously)

There's no visible pointer: TimeToString() returns cString, not cString*.
What looks like a dereference is really cString::operator*().

The object is in temporary storage somewhere. It goes out of scope just after
the isyslog() call is completed.

> cString strescape(const char *s, const char *chars)
> {
>   char *buffer;
>   const char *p = s;
>   char *t = NULL;
>   while (*p) {
>         if (strchr(chars, *p)) {
>            if (!t) {
>               buffer = MALLOC(char, 2 * strlen(s) + 1);
>               t = buffer + (p - s);
[snip]
>   return cString(s, t != NULL);
> }

> What happens if the malloc fails?
> VDR will coredump because of the "*t++"

That is likely, but it could happen even if the malloc succeeded if the
system is really short of memory - though this depends on the overcommit
setting, as exposed in /proc/sys/vm/overcommit_memory or the equivalent
sysctl. See the kernel documentation for more information.

> by intention?

Probably not :-)

> Pardon again the stupid question: where/how is that memory freed?

In this case, when the cString object is deleted - have a look at its
constructor and destructor.

> recording.c:

> void cRecordingUserCommand::InvokeCommand(const char *State, const char *RecordingFileName)
> {
>   if (command) {
>      char *cmd;
>      asprintf(&cmd, "%s %s \"%s\"", command, State, *strescape(RecordingFileName, "\"$"));
>      isyslog("executing '%s'", cmd);
>      SystemExec(cmd);
>      free(cmd);
>      }
> }

> Does asprintf know it can release the memory?

The content of cmd (on entry) is undefined and is ignored.

(BTW, I'm not a C++ expert either.)

-- 
| Darren Salt | nr. Ashington, | d youmustbejoking,demon,co,uk
| Debian,     | Northumberland | s zap,tartarus,org
| RISC OS     | Toon Army      | @                      Say NO to UK ID cards
|                                                       http://www.no2id.net/

Never be first. Never be last. Hope that there are more than two entrants.