Update patch set 3

Patch Set 3: (9 comments) Patch-set: 3
2024-02-14 13:55:11 +00:00 · 2024-02-14 13:55:11 +00:00 · 3f64bbafa0
parent a22626c86d
commit 3f64bbafa0
1 changed files with 157 additions and 0 deletions
--- a/157
+++ b/157
@ -0,0 +1,157 @@
+{
+  "comments": [
+    {
+      "unresolved": true,
+      "key": {
+        "uuid": "1dea7cd6_cc14eb33",
+        "filename": "include/osmocom/sigtran/osmo_ss7.h",
+        "patchSetId": 3
+      },
+      "lineNbr": 293,
+      "author": {
+        "id": 1000074
+      },
+      "writtenOn": "2024-02-14T13:55:11Z",
+      "side": 1,
+      "message": "does this need to be in the public header?",
+      "revId": "dacf17334861dd8d9531c4030742084b58d2aad4",
+      "serverId": "035e6965-6537-41bd-912c-053f3cf69326"
+    },
+    {
+      "unresolved": true,
+      "key": {
+        "uuid": "5ebb016a_373031ba",
+        "filename": "include/osmocom/sigtran/osmo_ss7.h",
+        "patchSetId": 3
+      },
+      "lineNbr": 349,
+      "author": {
+        "id": 1000074
+      },
+      "writtenOn": "2024-02-14T13:55:11Z",
+      "side": 1,
+      "message": "I\u0027m not sure this is really needed. You can keep an m3ua AS regardless of whether their ASP use TCP or SCTP. It could even have mixed ASPs some using TCp and some SCTP.",
+      "revId": "dacf17334861dd8d9531c4030742084b58d2aad4",
+      "serverId": "035e6965-6537-41bd-912c-053f3cf69326"
+    },
+    {
+      "unresolved": true,
+      "key": {
+        "uuid": "6183e0c1_8b6d0fd3",
+        "filename": "src/osmo_ss7_asp.c",
+        "patchSetId": 3
+      },
+      "lineNbr": 137,
+      "author": {
+        "id": 1000074
+      },
+      "writtenOn": "2024-02-14T13:55:11Z",
+      "side": 1,
+      "message": "you are assuming the value of IPPROTO_* is less than 256 here, which may be wrong.\nThis needs to be rewritten.",
+      "revId": "dacf17334861dd8d9531c4030742084b58d2aad4",
+      "serverId": "035e6965-6537-41bd-912c-053f3cf69326"
+    },
+    {
+      "unresolved": true,
+      "key": {
+        "uuid": "98844271_a11de137",
+        "filename": "src/osmo_ss7_asp.c",
+        "patchSetId": 3
+      },
+      "lineNbr": 532,
+      "author": {
+        "id": 1000074
+      },
+      "writtenOn": "2024-02-14T13:55:11Z",
+      "side": 1,
+      "message": "more like \"not supported\" maybe.",
+      "revId": "dacf17334861dd8d9531c4030742084b58d2aad4",
+      "serverId": "035e6965-6537-41bd-912c-053f3cf69326"
+    },
+    {
+      "unresolved": true,
+      "key": {
+        "uuid": "76daaf4e_92ee772e",
+        "filename": "src/osmo_ss7_asp.c",
+        "patchSetId": 3
+      },
+      "lineNbr": 887,
+      "author": {
+        "id": 1000074
+      },
+      "writtenOn": "2024-02-14T13:55:11Z",
+      "side": 1,
+      "message": "You didn\u0027t merge these 2 functions yet afaict. See previous comment son previous versions of this patch.",
+      "revId": "dacf17334861dd8d9531c4030742084b58d2aad4",
+      "serverId": "035e6965-6537-41bd-912c-053f3cf69326"
+    },
+    {
+      "unresolved": true,
+      "key": {
+        "uuid": "7fca15e0_a8d182d7",
+        "filename": "src/osmo_ss7_vty.c",
+        "patchSetId": 3
+      },
+      "lineNbr": 65,
+      "author": {
+        "id": 1000074
+      },
+      "writtenOn": "2024-02-14T13:55:11Z",
+      "side": 1,
+      "message": "This looks weird (and it think you found it fails). I\u0027d write: \"[(sctp|tcp)]\"\n\nA conditional param which can contain an sctp or tcp keyword.\nOtherwise it\u0027s like: A non-conditional param which can contain a conditional keyword which can be sctp or tcp.",
+      "revId": "dacf17334861dd8d9531c4030742084b58d2aad4",
+      "serverId": "035e6965-6537-41bd-912c-053f3cf69326"
+    },
+    {
+      "unresolved": true,
+      "key": {
+        "uuid": "43cf723e_891d8fca",
+        "filename": "src/osmo_ss7_vty.c",
+        "patchSetId": 3
+      },
+      "lineNbr": 995,
+      "author": {
+        "id": 1000074
+      },
+      "writtenOn": "2024-02-14T13:55:11Z",
+      "side": 1,
+      "message": "Why don\u0027t have \"transport-protocol (sctp|tcp)\" command around here?\nIs it really needed to set the transport type already in the \"asp NAME ...\" command?\n\nYou are missing a tcp-role (client|server) command btw.",
+      "revId": "dacf17334861dd8d9531c4030742084b58d2aad4",
+      "serverId": "035e6965-6537-41bd-912c-053f3cf69326"
+    },
+    {
+      "unresolved": true,
+      "key": {
+        "uuid": "32c77c78_579936fe",
+        "filename": "tests/vty/ss7_asp_test.vty",
+        "patchSetId": 3
+      },
+      "lineNbr": 58,
+      "author": {
+        "id": 1000074
+      },
+      "writtenOn": "2024-02-14T13:55:11Z",
+      "side": 1,
+      "message": "See, this looks weird. It should be the other way around: [(tcp|sctp)].",
+      "revId": "dacf17334861dd8d9531c4030742084b58d2aad4",
+      "serverId": "035e6965-6537-41bd-912c-053f3cf69326"
+    },
+    {
+      "unresolved": true,
+      "key": {
+        "uuid": "86d27b54_4f0ff9d9",
+        "filename": "tests/vty/ss7_asp_test.vty",
+        "patchSetId": 3
+      },
+      "lineNbr": 414,
+      "author": {
+        "id": 1000074
+      },
+      "writtenOn": "2024-02-14T13:55:11Z",
+      "side": 1,
+      "message": "my initial idea was to have a \"transport-protocol (tcp|sctp)\" in here instead. I\u0027m not blocking the other way if there are good reasons though.",
+      "revId": "dacf17334861dd8d9531c4030742084b58d2aad4",
+      "serverId": "035e6965-6537-41bd-912c-053f3cf69326"
+    }
+  ]
+}