From patchwork Thu Jun 30 09:59:19 2005 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Wolfgang Rohdewald X-Patchwork-Id: 11928 Received: from natsmtp00.rzone.de ([81.169.145.165]) by www.linuxtv.org with esmtp (Exim 4.34) id 1DnvpQ-0005u9-PW for vdr@linuxtv.org; Thu, 30 Jun 2005 11:59:28 +0200 Received: from wr.rohdewald.de (p548F8FAC.dip0.t-ipconnect.de [84.143.143.172]) (authenticated bits=0) by post.webmailer.de (8.13.1/8.13.1) with ESMTP id j5U9xRGx001117 for ; Thu, 30 Jun 2005 11:59:27 +0200 (MEST) Received: by wr.rohdewald.de (Postfix, from userid 107) id 1686A270022; Thu, 30 Jun 2005 11:59:27 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by wr.rohdewald.de (Postfix) with ESMTP id ECC9127001E for ; Thu, 30 Jun 2005 11:59:20 +0200 (CEST) From: Wolfgang Rohdewald To: vdr@linuxtv.org Subject: Re: [vdr] [PATCH] fix segfault in cSkins::Message Date: Thu, 30 Jun 2005 11:59:19 +0200 User-Agent: KMail/1.7.2 References: <200506301031.10756.wolfgang@rohdewald.de> In-Reply-To: <200506301031.10756.wolfgang@rohdewald.de> MIME-Version: 1.0 Message-Id: <200506301159.20072.wolfgang@rohdewald.de> X-Spam-Checker-Version: SpamAssassin 3.0.4 (2005-06-05) on wr.rohdewald.de X-Spam-Level: X-Spam-Status: No, score=-2.8 required=5.0 tests=ALL_TRUSTED,AWL autolearn=ham version=3.0.4 X-BeenThere: vdr@linuxtv.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: wolfgang@rohdewald.de, Klaus Schmidinger's VDR List-Id: Klaus Schmidinger's VDR List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 30 Jun 2005 09:59:28 -0000 Status: O X-Status: X-Keywords: X-UID: 3314 On Donnerstag 30 Juni 2005 10:31, Wolfgang Rohdewald wrote: > this happens if the MENUTIMEOUT makes the OSD menu > disappear while a message is displayed. more is needed - all delete osd should also nullify it. See gdb output below. also, I can only get rid of the segfault shown by valgrind below if constructors with osd as private member nullify osd right at the beginning, before calling NewOsd(). (I did not check which constructor causes my segfault, just applied it to all of them). I suppose other skins might have the same problem. This looks to me as if access to osd might not be threadsafe, but I am by no means a thread expert Extended patch attached. While being at it, I replaced a few more delete x by DELETENULL(x) Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 16384 (LWP 8769)] 0x080fec70 in cSkinClassicDisplayMessage::Flush (this=0x9f38ec0) at skinclassic.c:644 644 osd->Flush(); Current language: auto; currently c++ (gdb) p osd $1 = (class cOsd *) 0x10 (gdb) bt #0 0x080fec70 in cSkinClassicDisplayMessage::Flush (this=0x9f38ec0) at skinclassic.c:644 #1 0x081015dc in cSkins::Flush (this=0x81ef5a4) at skins.c:210 #2 0x080b95ab in cInterface::GetKey (this=0x9e5b6b8, Wait=true) at interface.c:35 #3 0x081223c5 in main (argc=14, argv=0xbf85ffd4) at vdr.c:639 line 644 is: if (osd) osd->Flush(); ==20735== Conditional jump or move depends on uninitialised value(s) ==20735== at 0x80FED8D: cSkinClassicDisplayMessage::Flush() (skinclassic.c:644) ==20735== by 0x81016FF: cSkins::Flush() (skins.c:210) ==20735== by 0x80B9612: cInterface::GetKey(bool) (interface.c:35) ==20735== by 0x81224E8: main (vdr.c:639) ==20735== ==20735== Use of uninitialised value of size 4 ==20735== at 0x80FED95: cSkinClassicDisplayMessage::Flush() (skinclassic.c:645) ==20735== by 0x81016FF: cSkins::Flush() (skins.c:210) ==20735== by 0x80B9612: cInterface::GetKey(bool) (interface.c:35) ==20735== by 0x81224E8: main (vdr.c:639) ==20735== ==20735== Invalid read of size 4 ==20735== at 0x80FEDA3: cSkinClassicDisplayMessage::Flush() (skinclassic.c:645) ==20735== by 0x81016FF: cSkins::Flush() (skins.c:210) ==20735== by 0x80B9612: cInterface::GetKey(bool) (interface.c:35) ==20735== by 0x81224E8: main (vdr.c:639) ==20735== Address 0x6564698A is not stack'd, malloc'd or (recently) free'd ==20735== ==20735== Process terminating with default action of signal 11 (SIGSEGV) ==20735== GPF (Pointer out of bounds?) ==20735== at 0x80FEDA3: cSkinClassicDisplayMessage::Flush() (skinclassic.c:645) ==20735== by 0x81016FF: cSkins::Flush() (skins.c:210) ==20735== by 0x80B9612: cInterface::GetKey(bool) (interface.c:35) ==20735== by 0x81224E8: main (vdr.c:639) ==20735== diff -up org27/dvbspu.c src/dvbspu.c --- org27/dvbspu.c 2005-05-07 13:13:48.000000000 +0200 +++ src/dvbspu.c 2005-06-30 11:12:53.000000000 +0200 @@ -235,9 +235,9 @@ cDvbSpuDecoder::cDvbSpuDecoder() cDvbSpuDecoder::~cDvbSpuDecoder() { - delete spubmp; - delete spu; - delete osd; + DELETENULL(spubmp); + DELETENULL(spu); + DELETENULL(osd); } void cDvbSpuDecoder::processSPU(uint32_t pts, uint8_t * buf, bool AllowedShow) @@ -246,8 +246,7 @@ void cDvbSpuDecoder::processSPU(uint32_t DEBUG("SPU pushData: pts: %d\n", pts); - delete spubmp; - spubmp = NULL; + DELETENULL(spubmp); delete[]spu; spu = buf; spupts = pts; @@ -390,16 +389,14 @@ void cDvbSpuDecoder::Draw(void) void cDvbSpuDecoder::Hide(void) { - delete osd; - osd = NULL; + DELETENULL(osd); } void cDvbSpuDecoder::Empty(void) { Hide(); - delete spubmp; - spubmp = NULL; + DELETENULL(spubmp); delete[]spu; spu = NULL; diff -up org27/skinclassic.c src/skinclassic.c --- org27/skinclassic.c 2005-05-16 12:45:07.000000000 +0200 +++ src/skinclassic.c 2005-06-30 11:30:55.000000000 +0200 @@ -88,6 +88,7 @@ public: cSkinClassicDisplayChannel::cSkinClassicDisplayChannel(bool WithInfo) { + osd = NULL; int Lines = WithInfo ? 5 : 1; const cFont *font = cFont::GetFont(fontOsd); lineHeight = font->Height(); @@ -101,7 +102,7 @@ cSkinClassicDisplayChannel::cSkinClassic cSkinClassicDisplayChannel::~cSkinClassicDisplayChannel() { - delete osd; + DELETENULL(osd); } void cSkinClassicDisplayChannel::SetChannel(const cChannel *Channel, int Number) @@ -174,6 +175,7 @@ public: cSkinClassicDisplayMenu::cSkinClassicDisplayMenu(void) { + osd = NULL; const cFont *font = cFont::GetFont(fontOsd); lineHeight = font->Height(); x0 = 0; @@ -200,7 +202,7 @@ cSkinClassicDisplayMenu::cSkinClassicDis cSkinClassicDisplayMenu::~cSkinClassicDisplayMenu() { - delete osd; + DELETENULL(osd); } void cSkinClassicDisplayMenu::SetScrollbar(void) @@ -394,6 +396,7 @@ public: cSkinClassicDisplayReplay::cSkinClassicDisplayReplay(bool ModeOnly) { + osd = NULL; const cFont *font = cFont::GetFont(fontOsd); int lineHeight = font->Height(); lastCurrentWidth = 0; @@ -412,7 +415,7 @@ cSkinClassicDisplayReplay::cSkinClassicD cSkinClassicDisplayReplay::~cSkinClassicDisplayReplay() { - delete osd; + DELETENULL(osd); } void cSkinClassicDisplayReplay::SetTitle(const char *Title) @@ -494,6 +497,7 @@ public: cSkinClassicDisplayVolume::cSkinClassicDisplayVolume(void) { + osd = NULL; const cFont *font = cFont::GetFont(fontOsd); int lineHeight = font->Height(); osd = cOsdProvider::NewOsd(Setup.OSDLeft, Setup.OSDTop + Setup.OSDHeight - lineHeight); @@ -503,7 +507,7 @@ cSkinClassicDisplayVolume::cSkinClassicD cSkinClassicDisplayVolume::~cSkinClassicDisplayVolume() { - delete osd; + DELETENULL(osd); } void cSkinClassicDisplayVolume::SetVolume(int Current, int Total, bool Mute) @@ -548,6 +552,7 @@ public: cSkinClassicDisplayTracks::cSkinClassicDisplayTracks(const char *Title, int NumTracks, const char * const *Tracks) { + osd = NULL; const cFont *font = cFont::GetFont(fontOsd); lineHeight = font->Height(); currentIndex = -1; @@ -575,7 +580,7 @@ cSkinClassicDisplayTracks::cSkinClassicD cSkinClassicDisplayTracks::~cSkinClassicDisplayTracks() { - delete osd; + DELETENULL(osd); } void cSkinClassicDisplayTracks::SetItem(const char *Text, int Index, bool Current) @@ -621,6 +626,7 @@ public: cSkinClassicDisplayMessage::cSkinClassicDisplayMessage(void) { + osd = NULL; const cFont *font = cFont::GetFont(fontOsd); int lineHeight = font->Height(); osd = cOsdProvider::NewOsd(Setup.OSDLeft, Setup.OSDTop + Setup.OSDHeight - lineHeight); @@ -630,7 +636,7 @@ cSkinClassicDisplayMessage::cSkinClassic cSkinClassicDisplayMessage::~cSkinClassicDisplayMessage() { - delete osd; + DELETENULL(osd); } void cSkinClassicDisplayMessage::SetMessage(eMessageType Type, const char *Text) @@ -641,7 +647,8 @@ void cSkinClassicDisplayMessage::SetMess void cSkinClassicDisplayMessage::Flush(void) { - osd->Flush(); + if (osd) + osd->Flush(); } // --- cSkinClassic ---------------------------------------------------------- Only in src: skinclassic.o diff -up org27/skins.c src/skins.c --- org27/skins.c 2005-01-14 14:07:19.000000000 +0100 +++ src/skins.c 2005-06-30 10:17:10.000000000 +0200 @@ -189,7 +191,8 @@ eKeys cSkins::Message(eMessageType Type, cStatus::MsgOsdClear(); } else { - cSkinDisplay::Current()->SetMessage(Type, NULL); + if (cSkinDisplay::Current()) + cSkinDisplay::Current()->SetMessage(Type, NULL); cStatus::MsgOsdStatusMessage(NULL); } }