Opened 4 years ago
Closed 4 years ago
#13528 closed Bug Report - General (fixed)
DVBChannel::ReturnMasterLock violates constness of variable
Reported by: | David Hampton | Owned by: | Klaas de Waal |
---|---|---|---|
Priority: | minor | Milestone: | 31.0 |
Component: | MythTV - DVB | Version: | Master Head |
Severity: | low | Keywords: | |
Cc: | Ticket locked: | no |
Description
There are two version of DVBChannel::ReturnMasterLock? in the code, the second taking a const pointer. It then casts away the constness of the variable and calls a function that appears to modify the variable. This is undefined behavior according to the standard.
Removing the const_cast and changing the parameter to be non-const requires a cascading series of changes to functions and parameters. This should be looked at by someone familiar with the DVB code.
Attachments (1)
Change History (5)
comment:1 Changed 4 years ago by
comment:2 Changed 4 years ago by
Owner: | set to Klaas de Waal |
---|---|
Severity: | medium → low |
Status: | new → assigned |
Changed 4 years ago by
Attachment: | 20191204-masterlock.patch added |
---|
Single implementation of GetMasterLock? and ReturnMasterLock?
comment:3 Changed 4 years ago by
The implementations of GetMasterLock? and ReturnMasterLock? in dvbchannel.cpp calls the functions with the same name in dtvchannel.cpp. Both versions of ReturnMasterLock? use the DVBChannel pointer returned by DTVChannel::GetMasterLock?. This function returns a normal, i.e. not const, value. So the constness is not a property of the object as declared but is added later. In that case it is allowed to use the const_cast to remove the constness.
Therefore the code is correct.
However, it is not elegant to have two implementations of GetMasterLock? and ReturnMasterLock? which are identical except for cast. The patch 20191204-masterlock.patch does remove a few const's and has only a single implementation of GetMasterLock? and ReturnMasterLock?.
comment:4 Changed 4 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
The code is indeed a bit strange. I will have a look at it. However, if the original author or somebody else who knows why this is coded the way it is can explain this that would be great.