From 8567077e373c19531d2abd5a0cc7aff15d171ac5 Mon Sep 17 00:00:00 2001 From: Gerrit User 1000004 <1000004@035e6965-6537-41bd-912c-053f3cf69326> Date: Tue, 20 Sep 2022 12:17:27 +0000 Subject: [PATCH] Update patch set 3 Patch Set 3: Code-Review-1 (3 comments) Patch-set: 3 Reviewer: Gerrit User 1000004 <1000004@035e6965-6537-41bd-912c-053f3cf69326> Label: Code-Review=-1 --- 4a8bd75e1e5f05a62291f7271d7e4c223d122e06 | 67 ++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 4a8bd75e1e5f05a62291f7271d7e4c223d122e06 diff --git a/4a8bd75e1e5f05a62291f7271d7e4c223d122e06 b/4a8bd75e1e5f05a62291f7271d7e4c223d122e06 new file mode 100644 index 0000000..c552fa7 --- /dev/null +++ b/4a8bd75e1e5f05a62291f7271d7e4c223d122e06 @@ -0,0 +1,67 @@ +{ + "comments": [ + { + "unresolved": true, + "key": { + "uuid": "33b82f9f_99caaf4d", + "filename": "src/tetra_upper_mac.c", + "patchSetId": 3 + }, + "lineNbr": 44, + "author": { + "id": 1000004 + }, + "writtenOn": "2022-09-20T12:17:27Z", + "side": 1, + "message": "this introduces a global, static variable. Normally all state should be contained witin the tetra_mac_state or whatever other related \u0027context\u0027 structure to ensure the code works also if there are many instances in parallel, without any clashes over global variables storing state that logically belongs to an instance.", + "range": { + "startLine": 44, + "startChar": 0, + "endLine": 44, + "endChar": 1 + }, + "revId": "4a8bd75e1e5f05a62291f7271d7e4c223d122e06", + "serverId": "035e6965-6537-41bd-912c-053f3cf69326" + }, + { + "unresolved": true, + "key": { + "uuid": "a2b9dee1_9d5a3db1", + "filename": "src/tetra_upper_mac.c", + "patchSetId": 3 + }, + "lineNbr": 50, + "author": { + "id": 1000004 + }, + "writtenOn": "2022-09-20T12:17:27Z", + "side": 1, + "message": "is there a specific reason to pre-allocate lots of message buffers here?", + "revId": "4a8bd75e1e5f05a62291f7271d7e4c223d122e06", + "serverId": "035e6965-6537-41bd-912c-053f3cf69326" + }, + { + "unresolved": true, + "key": { + "uuid": "48383659_ca5a1eda", + "filename": "src/tetra_upper_mac.h", + "patchSetId": 3 + }, + "lineNbr": 13, + "author": { + "id": 1000004 + }, + "writtenOn": "2022-09-20T12:17:27Z", + "side": 1, + "message": "sounds like \u0027active\u0027 could be of type bool?\nwould be nice to have some comments explaining the contents for the non-obvious ones such as \u0027age\u0027 and \u0027encrypytion\u0027.\n\nThe age comment should not just explain the units, but also explain the rationale of having a signed type for the age.\n\nFor \u0027encryption\u0027 it also would be good to explain what it is for. Is it just a flag? or an indication of a specific algorithm?... In the code I also see stuff like \u0027encryption \u003d encryption_mode\u0027 where the latter is an unsigned type. So once again the question arises why a signed type is used to represent something that\u0027s unsigned.", + "range": { + "startLine": 13, + "startChar": 0, + "endLine": 13, + "endChar": 3 + }, + "revId": "4a8bd75e1e5f05a62291f7271d7e4c223d122e06", + "serverId": "035e6965-6537-41bd-912c-053f3cf69326" + } + ] +} \ No newline at end of file