Problem with cChannel's copy constructor

Message ID 42F4A93F.8070109@cadsoft.de
State New
Headers

Commit Message

Klaus Schmidinger Aug. 6, 2005, 12:12 p.m. UTC
  Marcel Wiesweg wrote:
> Hi,
> 
> I just ran into a problem with cChannel's copy constructor.
> The problem is that this is actually not a copy constructor, because it sets 
> the values to 0 instead of copying, and - most important - it behaves 
> different from operator=.
> 
> It seems these two are not used in VDR except for one place (where I found 
> this), cSectionHandler::SetChannel. The value stored by cSectionHandle has 
> nid,tid,sid set to 0.
> In the line 
>   shp->channel = Channel ? *Channel : cChannel();
> there seems only the operator= involved, but actually the copy constructor is 
> invoked before the *Channel is given to opertator=.
> Attached is a short test program which illustrates the problem.
> 
> Solution is to make the copy constructor do copying.

The "copy constructor" is actually used intentionally in cChannels::NewChannel(),
where its sole purpose is to copy the actual transponder data.
But you're of course right, a copy constructor should behave like a real
copy constructor, and not serve a very special purpose.

I have therefore introduced cChannel::CopyTransponderData() for that purpose,
and changed the copy ctor so that it calls operator=.
See the attached patch and let me know if this works for you.

Klaus
  

Comments

Marcel Wiesweg Aug. 6, 2005, 6:08 p.m. UTC | #1
> The "copy constructor" is actually used intentionally in
> cChannels::NewChannel(), where its sole purpose is to copy the actual
> transponder data.
> But you're of course right, a copy constructor should behave like a real
> copy constructor, and not serve a very special purpose.
>
> I have therefore introduced cChannel::CopyTransponderData() for that
> purpose, and changed the copy ctor so that it calls operator=.
> See the attached patch and let me know if this works for you.

Yes, thanks, this works.

Marcel 
>
> Klaus
  

Patch

--- channels.h	2005/05/28 13:57:08	1.32
+++ channels.h	2005/08/06 11:23:32
@@ -181,6 +181,7 @@ 
   bool IsTerr(void) const { return cSource::IsTerr(source); }
   tChannelID GetChannelID(void) const { return tChannelID(source, nid, (nid || tid) ? tid : Transponder(), sid, rid); }
   int Modification(int Mask = CHANNELMOD_ALL);
+  void CopyTransponderData(const cChannel *Channel);
   bool SetSatTransponderData(int Source, int Frequency, char Polarization, int Srate, int CoderateH);
   bool SetCableTransponderData(int Source, int Frequency, int Modulation, int Srate, int CoderateH);
   bool SetTerrTransponderData(int Source, int Frequency, int Bandwidth, int Modulation, int Hierarchy, int CodeRateH, int CodeRateL, int Guard, int Transmission);
--- channels.c	2005/05/29 10:32:38	1.42
+++ channels.c	2005/08/06 12:06:37
@@ -179,27 +179,13 @@ 
 
 cChannel::cChannel(const cChannel &Channel)
 {
-  name = strdup("");
-  shortName = strdup("");
-  provider = strdup("");
-  portalName = strdup("");
-  *this = Channel;
-  vpid         = 0;
-  ppid         = 0;
-  apids[0]     = 0;
-  dpids[0]     = 0;
-  spids[0]     = 0;
-  tpid         = 0;
-  caids[0]     = 0;
-  nid          = 0;
-  tid          = 0;
-  sid          = 0;
-  rid          = 0;
-  number       = 0;
-  groupSep     = false;
-  modification = CHANNELMOD_NONE;
+  name = NULL;
+  shortName = NULL;
+  provider = NULL;
+  portalName = NULL;
   linkChannels = NULL;
   refChannel   = NULL;
+  *this = Channel;
 }
 
 cChannel::~cChannel()
@@ -265,6 +251,24 @@ 
   return Result;
 }
 
+void cChannel::CopyTransponderData(const cChannel *Channel)
+{
+  if (Channel) {
+     frequency    = Channel->frequency;
+     source       = Channel->source;
+     srate        = Channel->srate;
+     polarization = Channel->polarization;
+     inversion    = Channel->inversion;
+     bandwidth    = Channel->bandwidth;
+     coderateH    = Channel->coderateH;
+     coderateL    = Channel->coderateL;
+     modulation   = Channel->modulation;
+     transmission = Channel->transmission;
+     guard        = Channel->guard;
+     hierarchy    = Channel->hierarchy;
+     }
+}
+
 bool cChannel::SetSatTransponderData(int Source, int Frequency, char Polarization, int Srate, int CoderateH)
 {
   // Workarounds for broadcaster stupidity:
@@ -977,7 +981,8 @@ 
 {
   if (Transponder) {
      dsyslog("creating new channel '%s,%s;%s' on %s transponder %d with id %d-%d-%d-%d", Name, ShortName, Provider, *cSource::ToString(Transponder->Source()), Transponder->Transponder(), Nid, Tid, Sid, Rid);
-     cChannel *NewChannel = new cChannel(*Transponder);
+     cChannel *NewChannel = new cChannel;
+     NewChannel->CopyTransponderData(Transponder);
      NewChannel->SetId(Nid, Tid, Sid, Rid);
      NewChannel->SetName(Name, ShortName, Provider);
      Add(NewChannel);