[core] Apply PRE sockopt restriction in listening state (#1939)

* Docs: clarified the description of sockopt restrictions.
* Fixed FEC unit tests: call srt_setsockopt before srt_listen

docs/API/API-socket-options.md

@@ -122,13 +122,14 @@ of SRT ever created and put into use.
 2. **Restrict**: Defines restrictions on setting the option. The field is empty if the option
 is not settable (see **Dir** column):
 
-	- `pre-bind`: The option cannot be altered on a socket that is already bound (by calling
+    - `pre-bind`: The option cannot be altered on a socket that is already bound (by calling
 `srt_bind()` or any other function doing this, including automatic binding when trying to
-connect, as well as accepted sockets).
+connect, as well as accepted sockets). In other words, once an SRT socket has transitioned from
+`SRTS_INIT` to `SRTS_OPENED` socket state.
 
-    - `pre`: Like pre-bind, but only for a connected socket (including an accepted socket). If
-an option was set on a listener socket, it will be set the same on a socket returned by
-`srt_accept()`.
+    - `pre`: The option cannot be altered on a socket that is in `SRTS_LISTENING`, `SRTS_CONNECTING`
+or `SRTS_CONNECTED` state. If an option was set on a listener socket, it will be inherited
+by a socket returned by `srt_accept()` (except for `SRTO_STREAMID`).
 
     - `post`: The option is unrestricted and can be altered at any time, including when the
 socket is connected, as well as on an accepted socket. The setting of this flag on a listening 

srtcore/core.cpp

@@ -359,7 +359,7 @@ void CUDT::setOpt(SRT_SOCKOPT optName, const void* optval, int optlen)
     if (IsSet(oflags, SRTO_R_PREBIND) && m_bOpened)
         throw CUDTException(MJ_NOTSUP, MN_ISBOUND, 0);
 
-    if (IsSet(oflags, SRTO_R_PRE) && (m_bConnected || m_bConnecting))
+    if (IsSet(oflags, SRTO_R_PRE) && (m_bConnected || m_bConnecting || m_bListening))
         throw CUDTException(MJ_NOTSUP, MN_ISCONNECTED, 0);
 
     // Option execution. If this returns -1, there's no such option.

test/test_fec_rebuilding.cpp

@@ -287,14 +287,15 @@ TEST(TestFEC, Connection)
     ASSERT_EQ(inet_pton(AF_INET, "127.0.0.1", &sa.sin_addr), 1);
 
     srt_bind(l, (sockaddr*)& sa, sizeof(sa));
-    srt_listen(l, 1);
 
-    char fec_config1 [] = "fec,cols:10,rows:10";
-    char fec_config2 [] = "fec,cols:10,arq:never";
-    char fec_config_final [] = "fec,cols:10,rows:10,arq:never,layout:staircase";
+    const char fec_config1 [] = "fec,cols:10,rows:10";
+    const char fec_config2 [] = "fec,cols:10,arq:never";
+    const char fec_config_final [] = "fec,cols:10,rows:10,arq:never,layout:staircase";
 
     ASSERT_NE(srt_setsockflag(s, SRTO_PACKETFILTER, fec_config1, (sizeof fec_config1)-1), -1);
     ASSERT_NE(srt_setsockflag(l, SRTO_PACKETFILTER, fec_config2, (sizeof fec_config2)-1), -1);
+
+    srt_listen(l, 1);
 
     auto connect_res = std::async(std::launch::async, [&s, &sa]() {
         return srt_connect(s, (sockaddr*)& sa, sizeof(sa));
@@ -339,15 +340,16 @@ TEST(TestFEC, ConnectionReorder)
     ASSERT_EQ(inet_pton(AF_INET, "127.0.0.1", &sa.sin_addr), 1);
 
     srt_bind(l, (sockaddr*)& sa, sizeof(sa));
-    srt_listen(l, 1);
 
-    char fec_config1 [] = "fec,cols:10,rows:10";
-    char fec_config2 [] = "fec,rows:10,cols:10";
-    char fec_config_final [] = "fec,cols:10,rows:10,arq:onreq,layout:staircase";
+    const char fec_config1 [] = "fec,cols:10,rows:10";
+    const char fec_config2 [] = "fec,rows:10,cols:10";
+    const char fec_config_final [] = "fec,cols:10,rows:10,arq:onreq,layout:staircase";
 
     ASSERT_NE(srt_setsockflag(s, SRTO_PACKETFILTER, fec_config1, (sizeof fec_config1)-1), -1);
     ASSERT_NE(srt_setsockflag(l, SRTO_PACKETFILTER, fec_config2, (sizeof fec_config2)-1), -1);
 
+    srt_listen(l, 1);
+
     auto connect_res = std::async(std::launch::async, [&s, &sa]() {
         return srt_connect(s, (sockaddr*)& sa, sizeof(sa));
         });
@@ -391,15 +393,16 @@ TEST(TestFEC, ConnectionFull1)
     ASSERT_EQ(inet_pton(AF_INET, "127.0.0.1", &sa.sin_addr), 1);
 
     srt_bind(l, (sockaddr*)& sa, sizeof(sa));
-    srt_listen(l, 1);
 
-    char fec_config1 [] = "fec,cols:10,rows:20,arq:never,layout:even";
-    char fec_config2 [] = "fec,layout:even,rows:20,cols:10,arq:never";
-    char fec_config_final [] = "fec,cols:10,rows:20,arq:never,layout:even";
+    const char fec_config1 [] = "fec,cols:10,rows:20,arq:never,layout:even";
+    const char fec_config2 [] = "fec,layout:even,rows:20,cols:10,arq:never";
+    const char fec_config_final [] = "fec,cols:10,rows:20,arq:never,layout:even";
 
     ASSERT_NE(srt_setsockflag(s, SRTO_PACKETFILTER, fec_config1, (sizeof fec_config1)-1), -1);
     ASSERT_NE(srt_setsockflag(l, SRTO_PACKETFILTER, fec_config2, (sizeof fec_config2)-1), -1);
 
+    srt_listen(l, 1);
+
     auto connect_res = std::async(std::launch::async, [&s, &sa]() {
         return srt_connect(s, (sockaddr*)& sa, sizeof(sa));
         });
@@ -442,15 +445,16 @@ TEST(TestFEC, ConnectionFull2)
     ASSERT_EQ(inet_pton(AF_INET, "127.0.0.1", &sa.sin_addr), 1);
 
     srt_bind(l, (sockaddr*)& sa, sizeof(sa));
-    srt_listen(l, 1);
 
-    char fec_config1 [] = "fec,cols:10,rows:20,arq:always,layout:even";
-    char fec_config2 [] = "fec,layout:even,rows:20,cols:10,arq:always";
-    char fec_config_final [] = "fec,cols:10,rows:20,arq:always,layout:even";
+    const char fec_config1 [] = "fec,cols:10,rows:20,arq:always,layout:even";
+    const char fec_config2 [] = "fec,layout:even,rows:20,cols:10,arq:always";
+    const char fec_config_final [] = "fec,cols:10,rows:20,arq:always,layout:even";
 
     ASSERT_NE(srt_setsockflag(s, SRTO_PACKETFILTER, fec_config1, (sizeof fec_config1)-1), -1);
     ASSERT_NE(srt_setsockflag(l, SRTO_PACKETFILTER, fec_config2, (sizeof fec_config2)-1), -1);
 
+    srt_listen(l, 1);
+
     auto connect_res = std::async(std::launch::async, [&s, &sa]() {
         return srt_connect(s, (sockaddr*)& sa, sizeof(sa));
         });
@@ -494,15 +498,16 @@ TEST(TestFEC, ConnectionMess)
     ASSERT_EQ(inet_pton(AF_INET, "127.0.0.1", &sa.sin_addr), 1);
 
     srt_bind(l, (sockaddr*)& sa, sizeof(sa));
-    srt_listen(l, 1);
 
-    char fec_config1 [] = "fec,cols:,cols:10";
-    char fec_config2 [] = "fec,cols:,rows:10";
-    char fec_config_final [] = "fec,cols:10,rows:10,arq:onreq,layout:staircase";
+    const char fec_config1 [] = "fec,cols:,cols:10";
+    const char fec_config2 [] = "fec,cols:,rows:10";
+    const char fec_config_final [] = "fec,cols:10,rows:10,arq:onreq,layout:staircase";
 
     ASSERT_NE(srt_setsockflag(s, SRTO_PACKETFILTER, fec_config1, (sizeof fec_config1)-1), -1);
     ASSERT_NE(srt_setsockflag(l, SRTO_PACKETFILTER, fec_config2, (sizeof fec_config2)-1), -1);
 
+    srt_listen(l, 1);
+
     auto connect_res = std::async(std::launch::async, [&s, &sa]() {
         return srt_connect(s, (sockaddr*)& sa, sizeof(sa));
         });
@@ -546,13 +551,14 @@ TEST(TestFEC, ConnectionForced)
     ASSERT_EQ(inet_pton(AF_INET, "127.0.0.1", &sa.sin_addr), 1);
 
     srt_bind(l, (sockaddr*)& sa, sizeof(sa));
-    srt_listen(l, 1);
 
-    char fec_config1 [] = "fec,rows:20,cols:20";
-    char fec_config_final [] = "fec,cols:20,rows:20";
+    const char fec_config1 [] = "fec,rows:20,cols:20";
+    const char fec_config_final [] = "fec,cols:20,rows:20";
 
     ASSERT_NE(srt_setsockflag(s, SRTO_PACKETFILTER, fec_config1, (sizeof fec_config1)-1), -1);
 
+    srt_listen(l, 1);
+
     auto connect_res = std::async(std::launch::async, [&s, &sa]() {
         return srt_connect(s, (sockaddr*)& sa, sizeof(sa));
         });
@@ -592,14 +598,15 @@ TEST(TestFEC, RejectionConflict)
     ASSERT_EQ(inet_pton(AF_INET, "127.0.0.1", &sa.sin_addr), 1);
 
     srt_bind(l, (sockaddr*)& sa, sizeof(sa));
-    srt_listen(l, 1);
 
-    char fec_config1 [] = "fec,cols:10,rows:10";
-    char fec_config2 [] = "fec,cols:20,arq:never";
+    const char fec_config1 [] = "fec,cols:10,rows:10";
+    const char fec_config2 [] = "fec,cols:20,arq:never";
 
     srt_setsockflag(s, SRTO_PACKETFILTER, fec_config1, (sizeof fec_config1)-1);
     srt_setsockflag(l, SRTO_PACKETFILTER, fec_config2, (sizeof fec_config2)-1);
 
+    srt_listen(l, 1);
+
     auto connect_res = std::async(std::launch::async, [&s, &sa]() {
         return srt_connect(s, (sockaddr*)& sa, sizeof(sa));
         });
@@ -634,12 +641,12 @@ TEST(TestFEC, RejectionIncompleteEmpty)
     ASSERT_EQ(inet_pton(AF_INET, "127.0.0.1", &sa.sin_addr), 1);
 
     srt_bind(l, (sockaddr*)& sa, sizeof(sa));
-    srt_listen(l, 1);
-
-    char fec_config1 [] = "fec,rows:10";
 
+    const char fec_config1 [] = "fec,rows:10";
     srt_setsockflag(s, SRTO_PACKETFILTER, fec_config1, (sizeof fec_config1)-1);
 
+    srt_listen(l, 1);
+
     auto connect_res = std::async(std::launch::async, [&s, &sa]() {
         return srt_connect(s, (sockaddr*)& sa, sizeof(sa));
         });
@@ -675,14 +682,15 @@ TEST(TestFEC, RejectionIncomplete)
     ASSERT_EQ(inet_pton(AF_INET, "127.0.0.1", &sa.sin_addr), 1);
 
     srt_bind(l, (sockaddr*)& sa, sizeof(sa));
-    srt_listen(l, 1);
 
-    char fec_config1 [] = "fec,rows:10";
-    char fec_config2 [] = "fec,arq:never";
+    const char fec_config1 [] = "fec,rows:10";
+    const char fec_config2 [] = "fec,arq:never";
 
     srt_setsockflag(s, SRTO_PACKETFILTER, fec_config1, (sizeof fec_config1)-1);
     srt_setsockflag(l, SRTO_PACKETFILTER, fec_config2, (sizeof fec_config2)-1);
 
+    srt_listen(l, 1);
+
     auto connect_res = std::async(std::launch::async, [&s, &sa]() {
         return srt_connect(s, (sockaddr*)& sa, sizeof(sa));
         });
@@ -740,7 +748,7 @@ TEST_F(TestFECRebuilding, NoRebuild)
     SrtPacket fec_ctl(SRT_LIVE_MAX_PLSIZE);
 
     // Use the sequence number of the last packet, as usual.
-    bool have_fec_ctl = fec->packControlPacket(fec_ctl, seq);
+    const bool have_fec_ctl = fec->packControlPacket(fec_ctl, seq);
 
     ASSERT_EQ(have_fec_ctl, true);
     // By having all packets and FEC CTL packet, now stuff in
@@ -817,7 +825,7 @@ TEST_F(TestFECRebuilding, Rebuild)
     SrtPacket fec_ctl(SRT_LIVE_MAX_PLSIZE);
 
     // Use the sequence number of the last packet, as usual.
-    bool have_fec_ctl = fec->packControlPacket(fec_ctl, seq);
+    const bool have_fec_ctl = fec->packControlPacket(fec_ctl, seq);
 
     ASSERT_EQ(have_fec_ctl, true);
     // By having all packets and FEC CTL packet, now stuff in
@@ -863,7 +871,7 @@ TEST_F(TestFECRebuilding, Rebuild)
 
     // And now receive the FEC control packet
 
-    bool want_passthru_fec = fec->receive(*fecpkt, loss);
+    const bool want_passthru_fec = fec->receive(*fecpkt, loss);
     EXPECT_EQ(want_passthru_fec, false); // Confirm that it's been eaten up
 
     EXPECT_EQ(loss.size(), 0);