vdr-1.3.39 & crash bug in schedule

Message ID 43D0F002.6020103@cadsoft.de
State New
Headers

Commit Message

Klaus Schmidinger Jan. 20, 2006, 2:13 p.m. UTC
  Rolf Ahrenberg wrote:
> 
> Hi,
> 
> here's a little patch to prevent crashing on Digita network, that uses 
> 'NULL' in some event titles.

After some thought I believe it's probably better to do the following,
because an event should always have at least a title.

Klaus
  

Comments

Rolf Ahrenberg Jan. 20, 2006, 5:29 p.m. UTC | #1
On Fri, 20 Jan 2006, Klaus Schmidinger wrote:

> Rolf Ahrenberg wrote:
>> 
>> Hi,
>> 
>> here's a little patch to prevent crashing on Digita network, that uses 
>> 'NULL' in some event titles.
>
> After some thought I believe it's probably better to do the following,
> because an event should always have at least a title.

The functionality of your patch is exactly the same as in my 
vdr-1.3.3?-epg-bugfix.diff patches. However, I'm pretty sure I merged it 
before I started testing the 1.3.39, so does it really fix the problem? 
Should the fixup for title be done in cEvent::SetTitle() by changing 
'NULL' titles to " "?

BR,
--
rofa
  
Klaus Schmidinger Jan. 21, 2006, 1:21 p.m. UTC | #2
Rolf Ahrenberg wrote:
> On Fri, 20 Jan 2006, Klaus Schmidinger wrote:
> 
>> Rolf Ahrenberg wrote:
>>
>>>
>>> Hi,
>>>
>>> here's a little patch to prevent crashing on Digita network, that 
>>> uses 'NULL' in some event titles.
>>
>>
>> After some thought I believe it's probably better to do the following,
>> because an event should always have at least a title.
> 
> 
> The functionality of your patch is exactly the same as in my 
> vdr-1.3.3?-epg-bugfix.diff patches. However, I'm pretty sure I merged it 
> before I started testing the 1.3.39, so does it really fix the problem? 
> Should the fixup for title be done in cEvent::SetTitle() by changing 
> 'NULL' titles to " "?

Well, originally I wanted the events to be left exactly
as they came off the air if the bugfix level is 0.
However, events that don't even have a title make no
sense, so they shall always have one.

cEvent::SetTitle() is probably not a good place for this,
because it isn't guaranteed to be called at all.

The only real good place to do this would probably be the cEvent
constructor, but that would mean setting the title to something
that in 99.9% of all cases will be overwritten.

Since cEvent::FixEpgBugs() is called for all events that are
broadcast, it now makes sure every event actually has a non-NULL
title. The only loophole left is SVDRP's PUTE command, but the
man page states that "every event should at least have a T entry".
Maybe I should make that a "must".

Klaus
  
Rolf Ahrenberg Jan. 22, 2006, 6:16 p.m. UTC | #3
On Sat, 21 Jan 2006, Klaus Schmidinger wrote:

> Since cEvent::FixEpgBugs() is called for all events that are
> broadcast, it now makes sure every event actually has a non-NULL
> title. The only loophole left is SVDRP's PUTE command, but the

I'm not using any external EPG data and as I said in my earlier mail, 
the modification of FixEpgBugs() isn't enough and VDR 1.3.40 is crashing 
like before.

Fortunately my patch mentioned in the first message of this thread works 
quite nicely also with VDR 1.3.40, so finnish users can also enjoy the 
EPG search feature.

BR,
--
rofa
  
Klaus Schmidinger Jan. 22, 2006, 10:32 p.m. UTC | #4
Rolf Ahrenberg wrote:
> On Sat, 21 Jan 2006, Klaus Schmidinger wrote:
> 
>> Since cEvent::FixEpgBugs() is called for all events that are
>> broadcast, it now makes sure every event actually has a non-NULL
>> title. The only loophole left is SVDRP's PUTE command, but the
> 
> 
> I'm not using any external EPG data and as I said in my earlier mail, 
> the modification of FixEpgBugs() isn't enough and VDR 1.3.40 is crashing 
> like before.
> 
> Fortunately my patch mentioned in the first message of this thread works 
> quite nicely also with VDR 1.3.40, so finnish users can also enjoy the 
> EPG search feature.

I'm afraid I don't see why it would still crash.
You patched those places in menu.c where an event tile was
dereferenced. Now the code

  if (!title) {
      // we don't want any "(null)" titles
      title = strcpyrealloc(title, tr("No title"));
      EpgBugFixStat(12, ChannelID());
      }

in cEvent::FixEpgBugs() makes sure that every event has a
non-NULL title. So where exactly does it crash?

Klaus
  

Patch

--- epg.c       2006/01/20 13:42:38     1.50
+++ epg.c       2006/01/20 14:09:48
@@ -422,12 +422,17 @@ 
    strreplace(description, '\x86', ' ');
    strreplace(description, '\x87', ' ');

+  if (!title) {
+     // we don't want any "(null)" titles
+     title = strcpyrealloc(title, tr("No title"));
+     EpgBugFixStat(12, ChannelID());
+     }
+
    if (Setup.EPGBugfixLevel == 0)
       return;

    // Some TV stations apparently have their own idea about how to fill in the
    // EPG data. Let's fix their bugs as good as we can:
-  if (title) {

       // Some channels put the ShortText in quotes and use either the ShortText
       // or the Description field, depending on how long the string is:
@@ -609,12 +614,6 @@ 
              }
          }
       }
-  else {
-     // we don't want any "(null)" titles
-     title = strcpyrealloc(title, tr("No title"));
-     EpgBugFixStat(12, ChannelID());
-     }
-}

  // --- cSchedule -------------------------------------------------------------