diff mbox

*** glibc detected *** double free or corruption 1.4.2-1 Patch

Message ID 44FC5E13.7030301@cadsoft.de
State New
Headers

Commit Message

Klaus Schmidinger Sept. 4, 2006, 5:10 p.m. UTC 
  Udo Richter wrote:
> Udo Richter wrote:
>> ==4652== Invalid free() / delete / delete[]
>> ==4652==    at 0x1B904B04: free (vg_replace_malloc.c:152)
>> ==4652==    by 0x8103F5F: cTimer::operator=(cTimer const&) (timers.c:108)
>> ==4652==    by 0x80FE349: cSVDRP::CmdMODT(char const*) (svdrp.c:1136)
>> ==4652==    by 0x81015C1: cSVDRP::Process() (svdrp.c:1563)
>> ==4652==    by 0x80B3458: cInterface::GetKey(bool) (interface.c:37)
>> ==4652==    by 0x810D919: main (vdr.c:866)
>> ==4652==  Address 0x1BEEAC90 is 0 bytes inside a block of size 63 free'd
>> ==4652==    at 0x1B904B04: free (vg_replace_malloc.c:152)
>> ==4652==    by 0x8104D6F: cTimer::Parse(char const*) (timers.c:244)
>> ==4652==    by 0x80FE493: cSVDRP::CmdMODT(char const*) (svdrp.c:1132)
>> ==4652==    by 0x81015C1: cSVDRP::Process() (svdrp.c:1563)
>> ==4652==    by 0x80B3458: cInterface::GetKey(bool) (interface.c:37)
>> ==4652==    by 0x810D919: main (vdr.c:866)
> 
> 
> I think I've found it:
> 
> This is line 1127 of svdrp.c:
> 
>     cTimer t = *timer;
> 
> Although this looks like it calls cTimer::operator=, it actually calls 
> the default copy constructor of cTimer, because in this case = is not an 
> assignment, but an initialization. Because of that, the aux field is 
> used by both objects, thus the double free. Try this line to see if it 
> causes this:
> 
>     cTimer t;
>     t = *timer;

It's probably best to implement an actual copy-constructor.

Please try the attached patch, which contains both changes.

Klaus
diff mbox

Patch

--- timers.h	2006/04/08 12:41:44	1.28
+++ timers.h	2006/09/04 17:07:39
@@ -44,6 +44,7 @@ 
 public:
   cTimer(bool Instant = false, bool Pause = false, cChannel *Channel = NULL);
   cTimer(const cEvent *Event);
+  cTimer(const cTimer &Timer);
   virtual ~cTimer();
   cTimer& operator= (const cTimer &Timer);
   virtual int Compare(const cListObject &ListObject) const;
--- timers.c	2006/09/02 10:20:36	1.63
+++ timers.c	2006/09/04 17:08:32
@@ -83,6 +83,11 @@ 
   event = NULL; // let SetEvent() be called to get a log message
 }
 
+cTimer::cTimer(const cTimer &Timer)
+{
+  *this = Timer;
+}
+
 cTimer::~cTimer()
 {
   free(aux);
@@ -90,24 +95,26 @@ 
 
 cTimer& cTimer::operator= (const cTimer &Timer)
 {
-  startTime    = Timer.startTime;
-  stopTime     = Timer.stopTime;
-  lastSetEvent = 0;
-  recording    = Timer.recording;
-  pending      = Timer.pending;
-  inVpsMargin  = Timer.inVpsMargin;
-  flags        = Timer.flags;
-  channel      = Timer.channel;
-  day          = Timer.day;
-  weekdays     = Timer.weekdays;
-  start        = Timer.start;
-  stop         = Timer.stop;
-  priority     = Timer.priority;
-  lifetime     = Timer.lifetime;
-  strncpy(file, Timer.file, sizeof(file));
-  free(aux);
-  aux = Timer.aux ? strdup(Timer.aux) : NULL;
-  event = NULL;
+  if (&Timer == this) {
+     startTime    = Timer.startTime;
+     stopTime     = Timer.stopTime;
+     lastSetEvent = 0;
+     recording    = Timer.recording;
+     pending      = Timer.pending;
+     inVpsMargin  = Timer.inVpsMargin;
+     flags        = Timer.flags;
+     channel      = Timer.channel;
+     day          = Timer.day;
+     weekdays     = Timer.weekdays;
+     start        = Timer.start;
+     stop         = Timer.stop;
+     priority     = Timer.priority;
+     lifetime     = Timer.lifetime;
+     strncpy(file, Timer.file, sizeof(file));
+     free(aux);
+     aux = Timer.aux ? strdup(Timer.aux) : NULL;
+     event = NULL;
+     }
   return *this;
 }