Internal structures and basic management for socket groups #1109
Conversation
…/srt into dev-pre-group-api-complementary
…-group-api-complementary
There was a problem hiding this comment.
The set property naming property(value) does not follow the current naming convention.
It has to be changed to something like setProperty(value). In favor of faster merging, we'll probably create an issue for that and add it to the technical dept.
- Create an issue to rename the property function
srtcore/core.cpp
Outdated
| if (m_RcvBaseSeqNo == -1) | ||
| { | ||
| // One socket reported readiness, while no reading operation | ||
| // has ever been done. Whatever the sequence number is, it will | ||
| // be taken as a good deal and reading will be accepted. | ||
| ready = true; | ||
| } | ||
| else if ( (seqdiff = CSeqNo::seqcmp(sequence, m_RcvBaseSeqNo)) > 0 ) | ||
| { | ||
| // Case diff == 1: The very next. Surely read-ready. | ||
|
|
||
| // Case diff > 1: | ||
| // We have an ahead packet. There's one strict condition in which | ||
| // we may believe it needs to be delivered - when KANGAROO->HORSE | ||
| // transition is allowed. Stating that the time calculation is done | ||
| // exactly the same way on every link in the redundancy group, when | ||
| // it came to a situation that a packet from one link is ready for | ||
| // extraction while it has jumped over some packet, it has surely | ||
| // happened due to TLPKTDROP, and if it happened on at least one link, | ||
| // we surely don't have this packet ready on any other link. | ||
|
|
||
| // This might prove not exactly true, especially when at the moment | ||
| // when this happens another link may surprisinly receive this lacking | ||
| // packet, so the situation gets suddenly repaired after this function | ||
| // is called, the only result of it would be that it will really get | ||
| // the very next sequence, even though this function doesn't know it | ||
| // yet, but surely in both cases the situation is the same: the medium | ||
| // is ready for reading, no matter what packet will turn out to be | ||
| // returned when reading is done. | ||
|
|
||
| ready = true; | ||
| } |
There was a problem hiding this comment.
Will the body of both conditions contain more logic in further PRs? Right now they share the one and only statement.
There was a problem hiding this comment.
No, but mind that these two conditional expressions can't be bound into one anyway. And this comment is also important.
srtcore/core.cpp
Outdated
| int seqdiff = CSeqNo::seqcmp(a.sequence, m_RcvBaseSeqNo); | ||
| if ( seqdiff == 1) |
There was a problem hiding this comment.
const + no whitespaces inside of braces
srtcore/core.cpp
Outdated
| string CUDTGroup::StateStr(CUDTGroup::GroupState st) | ||
| { | ||
| static string states [] = { "PENDING", "IDLE", "RUNNING", "BROKEN" }; | ||
| if (int(st) < 5) | ||
| return states[st]; | ||
| return string("UNKNOWN"); | ||
| } |
There was a problem hiding this comment.
Isn't it better to return const char* to avoid copying?
There was a problem hiding this comment.
Yes, and potential crashes, too.
There was a problem hiding this comment.
You still return a string. instead of const char*
There was a problem hiding this comment.
UNKNOWN wasn't in the map - but still, should be possible...
This PR brings in the following changes:
CUDTGroupclass (some methods are removed also from the header in order to cut dependencies of management parts).CUDTSocketclass.CUDTclass that are strictly connected to operate with the group object.