Skip to content

Commit

Permalink
[core] Fixed reports from LGTM (#1542)
Browse files Browse the repository at this point in the history
  • Loading branch information
ethouris committed Oct 20, 2020
1 parent 724b841 commit e145ca5
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 27 deletions.
30 changes: 24 additions & 6 deletions apps/srt-tunnel.cpp
Expand Up @@ -115,17 +115,26 @@ class Medium
return os.str();
}

Medium(UriParser u, size_t ch): m_counter(s_counter++), m_uri(u), m_chunk(ch) {}
Medium(const UriParser& u, size_t ch): m_counter(s_counter++), m_uri(u), m_chunk(ch) {}
Medium(): m_counter(s_counter++) {}

virtual const char* type() = 0;
virtual bool IsOpen() = 0;
virtual void CloseInternal() = 0;

void Close()
void CloseState()
{
m_open = false;
m_broken = true;
}

// External API for this class that allows to close
// the entity on request. The CloseInternal should
// redirect to a type-specific function, the same that
// should be also called in destructor.
void Close()
{
CloseState();
CloseInternal();
}
virtual bool End() = 0;
Expand Down Expand Up @@ -169,6 +178,7 @@ class Medium

virtual ~Medium()
{
CloseState();
}

protected:
Expand Down Expand Up @@ -425,7 +435,7 @@ class SrtMedium: public Medium
bool End() override { return m_eof; }
bool Broken() override { return m_broken; }

void CloseInternal() override
void CloseSrt()
{
Verb() << "Closing SRT socket for " << uri();
lock_guard<mutex> lk(access);
Expand All @@ -435,6 +445,11 @@ class SrtMedium: public Medium
m_socket = SRT_ERROR;
}

// Forwarded in order to separate the implementation from
// the virtual function so that virtual function is not
// being called in destructor.
virtual void CloseInternal() override { return CloseSrt(); }

virtual const char* type() override { return "srt"; }
virtual int ReadInternal(char* output, int size) override;
virtual bool IsErrorAgain() override;
Expand All @@ -460,7 +475,8 @@ class SrtMedium: public Medium

virtual ~SrtMedium() override
{
Close();
CloseState();
CloseSrt();
}
};

Expand Down Expand Up @@ -509,7 +525,7 @@ class TcpMedium: public Medium
bool End() override { return m_eof; }
bool Broken() override { return m_broken; }

void CloseInternal() override
void CloseTcp()
{
Verb() << "Closing TCP socket for " << uri();
lock_guard<mutex> lk(access);
Expand All @@ -518,6 +534,7 @@ class TcpMedium: public Medium
tcp_close(m_socket);
m_socket = -1;
}
virtual void CloseInternal() { return CloseTcp(); }

virtual const char* type() override { return "tcp"; }
virtual int ReadInternal(char* output, int size) override;
Expand Down Expand Up @@ -552,7 +569,8 @@ class TcpMedium: public Medium

virtual ~TcpMedium()
{
Close();
CloseState();
CloseTcp();
}
};

Expand Down
2 changes: 1 addition & 1 deletion srtcore/cache.cpp
Expand Up @@ -47,7 +47,7 @@ written by

using namespace std;

CInfoBlock& CInfoBlock::operator=(const CInfoBlock& obj)
CInfoBlock& CInfoBlock::copyFrom(const CInfoBlock& obj)
{
std::copy(obj.m_piIP, obj.m_piIP + 4, m_piIP);
m_iIPversion = obj.m_iIPversion;
Expand Down
14 changes: 8 additions & 6 deletions srtcore/cache.h
Expand Up @@ -246,12 +246,14 @@ class CInfoBlock
double m_dCWnd; // congestion window size, congestion control

public:
virtual ~CInfoBlock() {}
virtual CInfoBlock& operator=(const CInfoBlock& obj);
virtual bool operator==(const CInfoBlock& obj);
virtual CInfoBlock* clone();
virtual int getKey();
virtual void release() {}
CInfoBlock() {} // NOTE: leaves uninitialized
CInfoBlock& copyFrom(const CInfoBlock& obj);
CInfoBlock(const CInfoBlock& src) { copyFrom(src); }
CInfoBlock& operator=(const CInfoBlock& src) { return copyFrom(src); }
bool operator==(const CInfoBlock& obj);
CInfoBlock* clone();
int getKey();
void release() {}

public:

Expand Down
5 changes: 4 additions & 1 deletion srtcore/common.h
Expand Up @@ -508,12 +508,15 @@ struct EventSlot
// "Stealing" copy constructor, following the auto_ptr method.
// This isn't very nice, but no other way to do it in C++03
// without rvalue-reference and move.
EventSlot(const EventSlot& victim)
void moveFrom(const EventSlot& victim)
{
slot = victim.slot; // Should MOVE.
victim.slot = 0;
}

EventSlot(const EventSlot& victim) { moveFrom(victim); }
EventSlot& operator=(const EventSlot& victim) { moveFrom(victim); return *this; }

EventSlot(void* op, EventSlotBase::dispatcher_t* disp)
{
slot = new SimpleEventSlot(op, disp);
Expand Down
2 changes: 1 addition & 1 deletion srtcore/core.cpp
Expand Up @@ -7385,7 +7385,7 @@ void CUDT::bstats(CBytePerfMon *perf, bool clear, bool instantaneous)

perf->mbpsMaxBW = m_llMaxBW > 0 ? Bps2Mbps(m_llMaxBW) : m_CongCtl.ready() ? Bps2Mbps(m_CongCtl->sndBandwidth()) : 0;

const uint32_t availbw = (uint64_t)(m_iBandwidth == 1 ? m_RcvTimeWindow.getBandwidth() : m_iBandwidth);
const int64_t availbw = m_iBandwidth == 1 ? m_RcvTimeWindow.getBandwidth() : m_iBandwidth;

perf->mbpsBandwidth = Bps2Mbps(availbw * (m_iMaxSRTPayloadSize + pktHdrSize));

Expand Down
30 changes: 18 additions & 12 deletions srtcore/group.h
Expand Up @@ -431,9 +431,9 @@ class CUDTGroup
{
static BufferedMessageStorage storage;

SRT_MSGCTRL mc;
char* data;
size_t size;
SRT_MSGCTRL mc;
mutable char* data;
size_t size;

BufferedMessage()
: data()
Expand All @@ -455,16 +455,22 @@ class CUDTGroup
memcpy(data, buf, s);
}

BufferedMessage(const BufferedMessage& foreign SRT_ATR_UNUSED)
: data()
, size()
BufferedMessage(const BufferedMessage& foreign)
: mc(foreign.mc)
, data(foreign.data)
, size(foreign.size)
{
foreign.data = 0;
}

BufferedMessage& operator=(const BufferedMessage& foreign)
{
// This is only to copy empty container.
// Any other use should not be done.
//#if ENABLE_DEBUG
// if (foreign.data)
// abort();
//#endif
data = foreign.data;
size = foreign.size;
mc = foreign.mc;

foreign.data = 0;
return *this;
}

private:
Expand Down
1 change: 1 addition & 0 deletions srtcore/packetfilter.h
Expand Up @@ -179,6 +179,7 @@ class PacketFilter
void receive(CUnit* unit, std::vector<CUnit*>& w_incoming, loss_seqs_t& w_loss_seqs);

protected:
PacketFilter& operator=(const PacketFilter& p);
void InsertRebuilt(std::vector<CUnit*>& incoming, CUnitQueue* uq);

CUDT* m_parent;
Expand Down

0 comments on commit e145ca5

Please sign in to comment.