)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1002042,"name":"Tim Nordell","email":"tim.nordell@nimbelink.com"},"change_message_id":"02d260d3e4dac786db5efcfdaf25f9cf5b0d5feb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d1ef29f8_4269c242","updated":"2022-09-12 20:02:37.000000000","message":"Thanks for reviewing!  I\u0027ll update the patchset accordingly - update incoming soonish.","commit_id":"96ba6023b0209fda45b3b9330f97dfd74abba774"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"728bef87299e736424186ce6e52f865460fd148c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"5ac86b7f_6889b53c","updated":"2022-09-13 21:50:01.000000000","message":"Thanks for the simplification.\nI\u0027m proposing some trivial extra simplification, let me know what is your idea about it","commit_id":"11fb91ba3d2e09813cafc438542a42c83db64de4"},{"author":{"_account_id":1002042,"name":"Tim Nordell","email":"tim.nordell@nimbelink.com"},"change_message_id":"810934b5927cf10896ece8f2ec426ef6c79bd62d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"db89ec79_c0fae277","updated":"2022-09-13 22:03:07.000000000","message":"Thanks.  I have a few comments below - I\u0027ll go ahead and roll back the loop back toward the initial implementation a bit.  Additionally, I\u0027ll move the if check set to an assert instead; logically we should never reach that state, so I probably should\u0027ve used that construct the first go around.","commit_id":"11fb91ba3d2e09813cafc438542a42c83db64de4"},{"author":{"_account_id":1002042,"name":"Tim Nordell","email":"tim.nordell@nimbelink.com"},"change_message_id":"8a2f85f547bea3d5b1afc9defbbd79d1d3a87a14","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"bbac76df_2828d3c7","updated":"2022-09-14 19:32:16.000000000","message":"\u003e Patch Set 7: Verified-1\n\u003e \n\u003e Build Failed \n\u003e \n\u003e https://build.openocd.org/job/openocd-gerrit-build/15928/ : FAILURE\n\u003e \n\u003e https://build.openocd.org/job/openocd-gerrit/16719/ : FAILURE\n\nBleh.  That\u0027s what I get for using `git add -p` and not doing a `git stash` afterwards before doing a local compile check.  (I locally had changed the verbosity of those printout statements that I didn\u0027t want committed since I didn\u0027t want to enable `-d3` since it\u0027s too verbose for what I was changing and didn\u0027t catch that those lines had other changes since I had done a find/replace to make the symbol names more readable.)","commit_id":"8bfa800e4d87e76f8c6861deefa9229e93bf1cc0"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"382fa57bbfcb45bd5041ed2ebae2b23e88b576a9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"13555fb1_e98876ab","updated":"2022-09-16 08:04:06.000000000","message":"I admit the original rtos_qsymbol() with re-looking up the symbol to get the next one is really awful.\nI would prefer gdb server has a function to register symbols to resolve and another one to return values after qSymbol packets exchange. It would ease qSymbol loop because per-gdb-connection indices could be used. It would also enable getting symbols from other OpenOCD parts than rtos.\nAll this is out of scope of this change.\n\nJust a couple of remarks to variable naming...","commit_id":"8bfa800e4d87e76f8c6861deefa9229e93bf1cc0"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"549b2a1c77232a45d1422fabeef6addecd4a8695","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"dac51111_e0b0483e","updated":"2022-09-14 21:17:50.000000000","message":"I think this version is readable and good enough.\nBut let\u0027s ask Tomas for a cross check","commit_id":"8bfa800e4d87e76f8c6861deefa9229e93bf1cc0"},{"author":{"_account_id":1002042,"name":"Tim Nordell","email":"tim.nordell@nimbelink.com"},"change_message_id":"4a552bfb866fe9659a89564be7df2e4ef37444a0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"b6feaf48_8f3a945f","in_reply_to":"13555fb1_e98876ab","updated":"2022-09-16 14:52:15.000000000","message":"Yeah, I agree; I\u0027m not a huge fan of the current organization of the scanning process, especially since it ends up doing a lot of unnecessary string comparisons (it looks like it\u0027s something on the order of O(n * log(n)) on the symbol lookups when it could be O(n) - not that we have that many symbols to lookup...) to match what symbol was just looked up.  It\u0027d be a better design if it could keep a reference to the last symbol tried instead as it goes so it could just increment as it scans.","commit_id":"8bfa800e4d87e76f8c6861deefa9229e93bf1cc0"},{"author":{"_account_id":1002042,"name":"Tim Nordell","email":"tim.nordell@nimbelink.com"},"change_message_id":"aecbdbd98ebe2b86c1d26e2d7d5af3f43b08d63b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"07e656fb_3fffacf7","in_reply_to":"dac51111_e0b0483e","updated":"2022-09-14 21:34:44.000000000","message":"Thanks.\n\nI also just realized the new include (\"helper/types.h\") isn\u0027t required anymore after the usage of ARRAY_SIZE(...) went away mid-series.  I\u0027ll wait for Tomas to review before removing that though.","commit_id":"8bfa800e4d87e76f8c6861deefa9229e93bf1cc0"},{"author":{"_account_id":1002042,"name":"Tim Nordell","email":"tim.nordell@nimbelink.com"},"change_message_id":"4a552bfb866fe9659a89564be7df2e4ef37444a0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"ac14b976_e8c27eac","updated":"2022-09-16 14:52:15.000000000","message":"Thanks for looking this over Antonio and Tomas!  I didn\u0027t specifically mention this, but I did test it against a project build that uses FreeRTOS in -flto mode in gcc, and it allows it to see the OS with this patch series.","commit_id":"0b1371d89f7cf25cc93fb7f0dac827cc946e4fb6"}],"src/rtos/rtos.c":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"5097d786a4e1ed9eee0750a4d3c56cf30a5ec75e","unresolved":true,"context_lines":[{"line_number":216,"context_line":" * If GDB replied about the last symbol for the RTOS and the RTOS was"},{"line_number":217,"context_line":" * specified explicitly, then no further symbol lookup is done. When"},{"line_number":218,"context_line":" * auto-detecting, the RTOS driver _detect() function must return success."},{"line_number":219,"context_line":" *"},{"line_number":220,"context_line":" * rtos_qsymbol() returns 1 if an RTOS has been detected, or 0 otherwise."},{"line_number":221,"context_line":" */"},{"line_number":222,"context_line":"int rtos_qsymbol(struct connection *connection, char const *packet, int packet_size)"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"9526fcdd_4e6597c6","line":219,"updated":"2022-09-07 23:29:11.000000000","message":"please add in the comment that you first try the symbol from RTOS, then again the same symbol with suffix \".lto_priv.0\" to handle the gcc flag \"-flto\"","commit_id":"96ba6023b0209fda45b3b9330f97dfd74abba774"},{"author":{"_account_id":1002042,"name":"Tim Nordell","email":"tim.nordell@nimbelink.com"},"change_message_id":"02d260d3e4dac786db5efcfdaf25f9cf5b0d5feb","unresolved":true,"context_lines":[{"line_number":216,"context_line":" * If GDB replied about the last symbol for the RTOS and the RTOS was"},{"line_number":217,"context_line":" * specified explicitly, then no further symbol lookup is done. When"},{"line_number":218,"context_line":" * auto-detecting, the RTOS driver _detect() function must return success."},{"line_number":219,"context_line":" *"},{"line_number":220,"context_line":" * rtos_qsymbol() returns 1 if an RTOS has been detected, or 0 otherwise."},{"line_number":221,"context_line":" */"},{"line_number":222,"context_line":"int rtos_qsymbol(struct connection *connection, char const *packet, int packet_size)"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"e493119f_9eb26f00","line":219,"in_reply_to":"9526fcdd_4e6597c6","updated":"2022-09-12 20:02:37.000000000","message":"Will do.","commit_id":"96ba6023b0209fda45b3b9330f97dfd74abba774"},{"author":{"_account_id":1002042,"name":"Tim Nordell","email":"tim.nordell@nimbelink.com"},"change_message_id":"4eaf7c54e63d2b01892281773720b00e7a88d8eb","unresolved":false,"context_lines":[{"line_number":216,"context_line":" * If GDB replied about the last symbol for the RTOS and the RTOS was"},{"line_number":217,"context_line":" * specified explicitly, then no further symbol lookup is done. When"},{"line_number":218,"context_line":" * auto-detecting, the RTOS driver _detect() function must return success."},{"line_number":219,"context_line":" *"},{"line_number":220,"context_line":" * rtos_qsymbol() returns 1 if an RTOS has been detected, or 0 otherwise."},{"line_number":221,"context_line":" */"},{"line_number":222,"context_line":"int rtos_qsymbol(struct connection *connection, char const *packet, int packet_size)"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"6923c1ad_6d5c8928","line":219,"in_reply_to":"e493119f_9eb26f00","updated":"2022-09-12 21:19:27.000000000","message":"Done","commit_id":"96ba6023b0209fda45b3b9330f97dfd74abba774"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"5097d786a4e1ed9eee0750a4d3c56cf30a5ec75e","unresolved":true,"context_lines":[{"line_number":247,"context_line":"\t/* Trim the appended lto_priv_suffix for all of the logic below */"},{"line_number":248,"context_line":"\tif (is_lto_priv)"},{"line_number":249,"context_line":"\t\tcur_sym[len - sizeof(lto_priv_suffix) + 1] \u003d 0;"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"\tif ((strcmp(packet, \"qSymbol::\") !\u003d 0) \u0026\u0026               /* GDB is not offering symbol lookup for the first time */"},{"line_number":252,"context_line":"\t    (!sscanf(packet, \"qSymbol:%\" SCNx64 \":\", \u0026addr))) { /* GDB did not find an address for a symbol */"},{"line_number":253,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":2,"id":"2db4cb7b_e1e17798","line":250,"updated":"2022-09-07 23:29:11.000000000","message":"I think the code is quite hard to read in your current proposal.\nI\u0027m trying to imagine a better code organization.\nI think that could be better removing \u0027retry_with_lto_priv\u0027 and using a \u0027const char *next_suffix;\u0027 that is set to \"\" or to lto_priv_suffix","commit_id":"96ba6023b0209fda45b3b9330f97dfd74abba774"},{"author":{"_account_id":1002042,"name":"Tim Nordell","email":"tim.nordell@nimbelink.com"},"change_message_id":"4eaf7c54e63d2b01892281773720b00e7a88d8eb","unresolved":false,"context_lines":[{"line_number":247,"context_line":"\t/* Trim the appended lto_priv_suffix for all of the logic below */"},{"line_number":248,"context_line":"\tif (is_lto_priv)"},{"line_number":249,"context_line":"\t\tcur_sym[len - sizeof(lto_priv_suffix) + 1] \u003d 0;"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"\tif ((strcmp(packet, \"qSymbol::\") !\u003d 0) \u0026\u0026               /* GDB is not offering symbol lookup for the first time */"},{"line_number":252,"context_line":"\t    (!sscanf(packet, \"qSymbol:%\" SCNx64 \":\", \u0026addr))) { /* GDB did not find an address for a symbol */"},{"line_number":253,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":2,"id":"9828771a_46f9766a","line":250,"in_reply_to":"102f19b3_00816deb","updated":"2022-09-12 21:19:27.000000000","message":"Done","commit_id":"96ba6023b0209fda45b3b9330f97dfd74abba774"},{"author":{"_account_id":1002042,"name":"Tim Nordell","email":"tim.nordell@nimbelink.com"},"change_message_id":"02d260d3e4dac786db5efcfdaf25f9cf5b0d5feb","unresolved":true,"context_lines":[{"line_number":247,"context_line":"\t/* Trim the appended lto_priv_suffix for all of the logic below */"},{"line_number":248,"context_line":"\tif (is_lto_priv)"},{"line_number":249,"context_line":"\t\tcur_sym[len - sizeof(lto_priv_suffix) + 1] \u003d 0;"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"\tif ((strcmp(packet, \"qSymbol::\") !\u003d 0) \u0026\u0026               /* GDB is not offering symbol lookup for the first time */"},{"line_number":252,"context_line":"\t    (!sscanf(packet, \"qSymbol:%\" SCNx64 \":\", \u0026addr))) { /* GDB did not find an address for a symbol */"},{"line_number":253,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":2,"id":"102f19b3_00816deb","line":250,"in_reply_to":"2db4cb7b_e1e17798","updated":"2022-09-12 20:02:37.000000000","message":"Yeah, I suppose that\u0027d make it a little easier.  Always treat it as having a suffix and modify the one to try instead.  I\u0027ll do that.\n\nIt would be easier, too, if the symbol lookup code wasn\u0027t stateless.  E.g. the current path is:\n\n1. Send a symbol query out\n2. Recognize which symbol the query matches from doing string comparisons against the response, and determine the next symbol from this response.\n\nIt\u0027d be a lot easier from a code legibility standpoint if there was at least a state saved somewhere.  E.g. something like:\n\nstruct symbol_lookup_state\n{\n    struct symbol_table_elem *cur;\n    unsigned int suffix_id;\n};\n\nthen you wouldn\u0027t have to strictly use a strcmp(...) to determine which symbol variant you\u0027re currently working with (other than to validate that the response from gdb matched what you were expecting), and it\u0027d remove the need to rewalk the OS symbol table element list *every* time a response came back from GDB as it does today.\n\nI put this additional check in without readjusting the overall architecture since my goal was the least impact to the codebase, but I could modify it overall to have a saved state instead.","commit_id":"96ba6023b0209fda45b3b9330f97dfd74abba774"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"728bef87299e736424186ce6e52f865460fd148c","unresolved":false,"context_lines":[{"line_number":247,"context_line":"\t/* Trim the appended lto_priv_suffix for all of the logic below */"},{"line_number":248,"context_line":"\tif (is_lto_priv)"},{"line_number":249,"context_line":"\t\tcur_sym[len - sizeof(lto_priv_suffix) + 1] \u003d 0;"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"\tif ((strcmp(packet, \"qSymbol::\") !\u003d 0) \u0026\u0026               /* GDB is not offering symbol lookup for the first time */"},{"line_number":252,"context_line":"\t    (!sscanf(packet, \"qSymbol:%\" SCNx64 \":\", \u0026addr))) { /* GDB did not find an address for a symbol */"},{"line_number":253,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":2,"id":"fbefc770_911473a2","line":250,"in_reply_to":"9828771a_46f9766a","updated":"2022-09-13 21:50:01.000000000","message":"\u003e It\u0027d be a lot easier from a code legibility standpoint if there was at least a state saved somewhere.\n\nIt could get complicated if we have multiple targets, each with its own GDB attached. The state should be saved in struct connection to decouple it per GDB.\nNot sure this will really helps overall readability.","commit_id":"96ba6023b0209fda45b3b9330f97dfd74abba774"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"5097d786a4e1ed9eee0750a4d3c56cf30a5ec75e","unresolved":true,"context_lines":[{"line_number":306,"context_line":"\t}"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"\tif (8 + (strlen(next_sym-\u003esymbol_name) * 2 + retry_with_lto_priv * strlen(lto_priv_suffix)) + 1 \u003e sizeof(reply)) {"},{"line_number":310,"context_line":"\t\tLOG_ERROR(\"ERROR: RTOS symbol \u0027%s\u0027 name is too long for GDB!\", next_sym-\u003esymbol_name);"},{"line_number":311,"context_line":"\t\tgoto done;"},{"line_number":312,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"c88ecb04_1483ecb9","line":309,"updated":"2022-09-07 23:29:11.000000000","message":"NO, \u0027retry_with_lto_priv\u0027 is of type \u0027bool\u0027.\nEven if C standard says that \u0027bool\u0027 is an \u0027int\u0027 and \u0027true\u0027 is 1, we do not use \u0027bool\u0027 types in computation!\nPlus the check is wrong, because the string is converted to hex and the computation misses 2x on strlen(lto_priv_suffix)!!\nUse something like:\n unsigned int check_len \u003d 8 + (strlen(next_sym-\u003esymbol_name) * 2) + 1;\n check_len +\u003d (retry_with_lto_priv) ? 2 * strlen(lto_priv_suffix) : 0;\n if (check_len \u003e sizeof(reply)) {","commit_id":"96ba6023b0209fda45b3b9330f97dfd74abba774"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"728bef87299e736424186ce6e52f865460fd148c","unresolved":false,"context_lines":[{"line_number":306,"context_line":"\t}"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"\tif (8 + (strlen(next_sym-\u003esymbol_name) * 2 + retry_with_lto_priv * strlen(lto_priv_suffix)) + 1 \u003e sizeof(reply)) {"},{"line_number":310,"context_line":"\t\tLOG_ERROR(\"ERROR: RTOS symbol \u0027%s\u0027 name is too long for GDB!\", next_sym-\u003esymbol_name);"},{"line_number":311,"context_line":"\t\tgoto done;"},{"line_number":312,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"81e54999_6b93c97a","line":309,"in_reply_to":"1ed94f1e_a59b94e1","updated":"2022-09-13 21:50:01.000000000","message":"yes, much more readable.\ndon\u0027t really worry about performance, here. The bottleneck is always the connection through JTAG and, eventually USB, not the computation on host side.","commit_id":"96ba6023b0209fda45b3b9330f97dfd74abba774"},{"author":{"_account_id":1002042,"name":"Tim Nordell","email":"tim.nordell@nimbelink.com"},"change_message_id":"4eaf7c54e63d2b01892281773720b00e7a88d8eb","unresolved":false,"context_lines":[{"line_number":306,"context_line":"\t}"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"\tif (8 + (strlen(next_sym-\u003esymbol_name) * 2 + retry_with_lto_priv * strlen(lto_priv_suffix)) + 1 \u003e sizeof(reply)) {"},{"line_number":310,"context_line":"\t\tLOG_ERROR(\"ERROR: RTOS symbol \u0027%s\u0027 name is too long for GDB!\", next_sym-\u003esymbol_name);"},{"line_number":311,"context_line":"\t\tgoto done;"},{"line_number":312,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"1ed94f1e_a59b94e1","line":309,"in_reply_to":"215d1de6_07d5344c","updated":"2022-09-12 21:19:27.000000000","message":"I modified this to calculate reply_len in the same format as below, basically, across multiple lines.  Should make it easy to compare the two for changes in the future.","commit_id":"96ba6023b0209fda45b3b9330f97dfd74abba774"},{"author":{"_account_id":1002042,"name":"Tim Nordell","email":"tim.nordell@nimbelink.com"},"change_message_id":"02d260d3e4dac786db5efcfdaf25f9cf5b0d5feb","unresolved":true,"context_lines":[{"line_number":306,"context_line":"\t}"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"\tif (8 + (strlen(next_sym-\u003esymbol_name) * 2 + retry_with_lto_priv * strlen(lto_priv_suffix)) + 1 \u003e sizeof(reply)) {"},{"line_number":310,"context_line":"\t\tLOG_ERROR(\"ERROR: RTOS symbol \u0027%s\u0027 name is too long for GDB!\", next_sym-\u003esymbol_name);"},{"line_number":311,"context_line":"\t\tgoto done;"},{"line_number":312,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"215d1de6_07d5344c","line":309,"in_reply_to":"c88ecb04_1483ecb9","updated":"2022-09-12 20:02:37.000000000","message":"You\u0027re right - I missed the * 2 here.  I\u0027ll adjust, and remove the boolean from the computation for code legibiity.\n\n(On some platforms it\u0027s an extremely minor optimization to use multiplication here as it removes a branch from the resulting code, although, depending on if it\u0027s a single cycle multiple or not, it might end up costing more.  Since this isn\u0027t really a core loop anywhere, this doesn\u0027t really matter.)","commit_id":"96ba6023b0209fda45b3b9330f97dfd74abba774"},{"author":{"_account_id":1002042,"name":"Tim Nordell","email":"tim.nordell@nimbelink.com"},"change_message_id":"4eaf7c54e63d2b01892281773720b00e7a88d8eb","unresolved":false,"context_lines":[{"line_number":262,"context_line":"\t\t\t/* A suffix length of 0 is special since it always matches; we want to"},{"line_number":263,"context_line":"\t\t\t * still attempt other suffixes for determining last_suffix and next_suffix. */"},{"line_number":264,"context_line":"\t\t\tif (0 !\u003d suffix_len)"},{"line_number":265,"context_line":"\t\t\t\tbreak;"},{"line_number":266,"context_line":"\t\t}"},{"line_number":267,"context_line":"\t}"},{"line_number":268,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":3,"id":"d42bf3ea_a8bfda5e","line":265,"updated":"2022-09-12 21:19:27.000000000","message":"Note: We wouldn\u0027t have needed this special case if \"\" was always at the end of the suffix_list[], but I wanted to protect against (potential) future breakage in case someone reordered suffix_list[...].  Right now I left it with \"\" as the first suffix to look for for the prior symbol lookup.\n\n(I still think overall it\u0027d be more legible if we didn\u0027t do this detection; I\u0027m not sure why this was stateless originally, but someone could replace this entire for loop here with stateful lookups instead in the future.  I didn\u0027t want to change the stateless vs stateful design without a further discussion.)","commit_id":"1410ff3ff3ac7bf35807c434f4d3e2912daa020a"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"728bef87299e736424186ce6e52f865460fd148c","unresolved":true,"context_lines":[{"line_number":234,"context_line":"\tstruct target *target \u003d get_target_from_connection(connection);"},{"line_number":235,"context_line":"\tstruct rtos *os \u003d target-\u003ertos;"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"\tstatic const char * const suffix_list[] \u003d {\"\", \".lto_priv.0\"};"},{"line_number":238,"context_line":"\tconst char *last_suffix \u003d NULL;"},{"line_number":239,"context_line":"\tconst char *next_suffix \u003d NULL;"},{"line_number":240,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":4,"id":"a657a92c_6581ed1f","line":237,"updated":"2022-09-13 21:50:01.000000000","message":"I don\u0027t expect this suffix_list[] to grow (please shout at me if I\u0027m wrong).\nSo this can be suffix_list[2] with some simplification below.","commit_id":"11fb91ba3d2e09813cafc438542a42c83db64de4"},{"author":{"_account_id":1002042,"name":"Tim Nordell","email":"tim.nordell@nimbelink.com"},"change_message_id":"810934b5927cf10896ece8f2ec426ef6c79bd62d","unresolved":true,"context_lines":[{"line_number":234,"context_line":"\tstruct target *target \u003d get_target_from_connection(connection);"},{"line_number":235,"context_line":"\tstruct rtos *os \u003d target-\u003ertos;"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"\tstatic const char * const suffix_list[] \u003d {\"\", \".lto_priv.0\"};"},{"line_number":238,"context_line":"\tconst char *last_suffix \u003d NULL;"},{"line_number":239,"context_line":"\tconst char *next_suffix \u003d NULL;"},{"line_number":240,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":4,"id":"adbd5345_f5202ffa","line":237,"in_reply_to":"a657a92c_6581ed1f","updated":"2022-09-13 22:03:07.000000000","message":"Correct - today it\u0027s only the two options and I don\u0027t foresee any other options right now.  I tend to prefer to write code that iterates over data containing our objectives (e.g. the two here) for future flexibility, but I\u0027d be okay unrolling the loop (which would be a middle ground back toward the initial version a bit) since it makes it simpler overall.","commit_id":"11fb91ba3d2e09813cafc438542a42c83db64de4"},{"author":{"_account_id":1002042,"name":"Tim Nordell","email":"tim.nordell@nimbelink.com"},"change_message_id":"a4e9b26dbeb60a0fd52ccbda7f4562747ddd30b8","unresolved":false,"context_lines":[{"line_number":234,"context_line":"\tstruct target *target \u003d get_target_from_connection(connection);"},{"line_number":235,"context_line":"\tstruct rtos *os \u003d target-\u003ertos;"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"\tstatic const char * const suffix_list[] \u003d {\"\", \".lto_priv.0\"};"},{"line_number":238,"context_line":"\tconst char *last_suffix \u003d NULL;"},{"line_number":239,"context_line":"\tconst char *next_suffix \u003d NULL;"},{"line_number":240,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":4,"id":"1b37e89e_7feafb33","line":237,"in_reply_to":"adbd5345_f5202ffa","updated":"2022-09-13 23:10:21.000000000","message":"Done","commit_id":"11fb91ba3d2e09813cafc438542a42c83db64de4"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"728bef87299e736424186ce6e52f865460fd148c","unresolved":true,"context_lines":[{"line_number":248,"context_line":"\tcur_sym[len] \u003d 0;"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"\t/* Detect what suffix was used during the previous symbol lookup attempt */"},{"line_number":251,"context_line":"\tfor (size_t i \u003d 0; i \u003c ARRAY_SIZE(suffix_list); ++i) {"},{"line_number":252,"context_line":"\t\tsize_t suffix_len \u003d strlen(suffix_list[i]);"},{"line_number":253,"context_line":"\t\tif (len \u003e suffix_len \u0026\u0026 !strcmp(cur_sym + len - suffix_len, suffix_list[i])) {"},{"line_number":254,"context_line":"\t\t\t/* Denote which suffix, and trim the cur_sym to just the symbol itself. */"}],"source_content_type":"text/x-csrc","patch_set":4,"id":"64b38aa3_3357ad27","line":251,"updated":"2022-09-13 21:50:01.000000000","message":"dropping the loop and knowing that suffix_list[0] always matches, you can only test for suffix_list[1]\n size_t suffix_len \u003d strlen(suffix_list[1]);\n if (len \u003e suffix_len \u0026\u0026 !strcmp(cur_sym + len - suffix_len, suffix_list[1])) {\n   cur_sym[len - suffix_len] \u003d \u0027\\0\u0027;\n   last_suffix \u003d suffix_list[1];\n   next_suffix \u003d NULL;\n } else {\n   last_suffix \u003d suffix_list[0];\n   next_suffix \u003d suffix_list[1];\n }","commit_id":"11fb91ba3d2e09813cafc438542a42c83db64de4"},{"author":{"_account_id":1002042,"name":"Tim Nordell","email":"tim.nordell@nimbelink.com"},"change_message_id":"a4e9b26dbeb60a0fd52ccbda7f4562747ddd30b8","unresolved":false,"context_lines":[{"line_number":248,"context_line":"\tcur_sym[len] \u003d 0;"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"\t/* Detect what suffix was used during the previous symbol lookup attempt */"},{"line_number":251,"context_line":"\tfor (size_t i \u003d 0; i \u003c ARRAY_SIZE(suffix_list); ++i) {"},{"line_number":252,"context_line":"\t\tsize_t suffix_len \u003d strlen(suffix_list[i]);"},{"line_number":253,"context_line":"\t\tif (len \u003e suffix_len \u0026\u0026 !strcmp(cur_sym + len - suffix_len, suffix_list[i])) {"},{"line_number":254,"context_line":"\t\t\t/* Denote which suffix, and trim the cur_sym to just the symbol itself. */"}],"source_content_type":"text/x-csrc","patch_set":4,"id":"198500df_53656d63","line":251,"in_reply_to":"419a5e57_2b05abda","updated":"2022-09-13 23:10:21.000000000","message":"Done","commit_id":"11fb91ba3d2e09813cafc438542a42c83db64de4"},{"author":{"_account_id":1002042,"name":"Tim Nordell","email":"tim.nordell@nimbelink.com"},"change_message_id":"810934b5927cf10896ece8f2ec426ef6c79bd62d","unresolved":true,"context_lines":[{"line_number":248,"context_line":"\tcur_sym[len] \u003d 0;"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"\t/* Detect what suffix was used during the previous symbol lookup attempt */"},{"line_number":251,"context_line":"\tfor (size_t i \u003d 0; i \u003c ARRAY_SIZE(suffix_list); ++i) {"},{"line_number":252,"context_line":"\t\tsize_t suffix_len \u003d strlen(suffix_list[i]);"},{"line_number":253,"context_line":"\t\tif (len \u003e suffix_len \u0026\u0026 !strcmp(cur_sym + len - suffix_len, suffix_list[i])) {"},{"line_number":254,"context_line":"\t\t\t/* Denote which suffix, and trim the cur_sym to just the symbol itself. */"}],"source_content_type":"text/x-csrc","patch_set":4,"id":"419a5e57_2b05abda","line":251,"in_reply_to":"64b38aa3_3357ad27","updated":"2022-09-13 22:03:07.000000000","message":"Agreed that it makes it a simpler check.  (And removes the special case for a suffix of length 0.)","commit_id":"11fb91ba3d2e09813cafc438542a42c83db64de4"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"728bef87299e736424186ce6e52f865460fd148c","unresolved":true,"context_lines":[{"line_number":323,"context_line":"\t}"},{"line_number":324,"context_line":""},{"line_number":325,"context_line":"\tif (!next_suffix)"},{"line_number":326,"context_line":"\t\tnext_suffix \u003d \"\";"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"\treply_len \u003d 8;                                   /* snprintf(..., \"qSymbol:\") */"},{"line_number":329,"context_line":"\treply_len +\u003d 2 * strlen(next_sym-\u003esymbol_name);  /* hexify(..., next_sym-\u003esymbol_name, ...) */"}],"source_content_type":"text/x-csrc","patch_set":4,"id":"751683f1_7e05109a","line":326,"updated":"2022-09-13 21:50:01.000000000","message":"next_suffix \u003d suffix_list[0]; ???","commit_id":"11fb91ba3d2e09813cafc438542a42c83db64de4"},{"author":{"_account_id":1002042,"name":"Tim Nordell","email":"tim.nordell@nimbelink.com"},"change_message_id":"a4e9b26dbeb60a0fd52ccbda7f4562747ddd30b8","unresolved":false,"context_lines":[{"line_number":323,"context_line":"\t}"},{"line_number":324,"context_line":""},{"line_number":325,"context_line":"\tif (!next_suffix)"},{"line_number":326,"context_line":"\t\tnext_suffix \u003d \"\";"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"\treply_len \u003d 8;                                   /* snprintf(..., \"qSymbol:\") */"},{"line_number":329,"context_line":"\treply_len +\u003d 2 * strlen(next_sym-\u003esymbol_name);  /* hexify(..., next_sym-\u003esymbol_name, ...) */"}],"source_content_type":"text/x-csrc","patch_set":4,"id":"7ba20c2d_f30fcc63","line":326,"in_reply_to":"5cf97c92_7908d81f","updated":"2022-09-13 23:10:21.000000000","message":"Done","commit_id":"11fb91ba3d2e09813cafc438542a42c83db64de4"},{"author":{"_account_id":1002042,"name":"Tim Nordell","email":"tim.nordell@nimbelink.com"},"change_message_id":"810934b5927cf10896ece8f2ec426ef6c79bd62d","unresolved":true,"context_lines":[{"line_number":323,"context_line":"\t}"},{"line_number":324,"context_line":""},{"line_number":325,"context_line":"\tif (!next_suffix)"},{"line_number":326,"context_line":"\t\tnext_suffix \u003d \"\";"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"\treply_len \u003d 8;                                   /* snprintf(..., \"qSymbol:\") */"},{"line_number":329,"context_line":"\treply_len +\u003d 2 * strlen(next_sym-\u003esymbol_name);  /* hexify(..., next_sym-\u003esymbol_name, ...) */"}],"source_content_type":"text/x-csrc","patch_set":4,"id":"5cf97c92_7908d81f","line":326,"in_reply_to":"751683f1_7e05109a","updated":"2022-09-13 22:03:07.000000000","message":"Yeah, maybe not strictly intuitive here.  I didn\u0027t want to pass in NULL into the various routines below, and it\u0027s safe to pass an empty string for that case.  Although, I guess, we shouldn\u0027t ever *reach* here with a next_suffix that is NULL.  How about this instead:\n\n   assert(next_suffix);\n\nThe code above should guarantee it\u0027s always set by the time it reaches this point in the code, and this should help protect against someone modifying the above code in a way that ends up with a NULL below.","commit_id":"11fb91ba3d2e09813cafc438542a42c83db64de4"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"382fa57bbfcb45bd5041ed2ebae2b23e88b576a9","unresolved":true,"context_lines":[{"line_number":243,"context_line":"\tsize_t len \u003d unhexify((uint8_t *)cur_sym, strchr(packet + 8, \u0027:\u0027) + 1, strlen(strchr(packet + 8, \u0027:\u0027) + 1));"},{"line_number":244,"context_line":"\tcur_sym[len] \u003d 0;"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"\tconst char first_suffix[] \u003d \"\";"},{"line_number":247,"context_line":"\tconst char lto_suffix[] \u003d \".lto_priv.0\";"},{"line_number":248,"context_line":"\tconst size_t lto_suffix_len \u003d strlen(lto_suffix);"},{"line_number":249,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":8,"id":"b15f64c1_98d8dcff","line":246,"range":{"start_line":246,"start_character":12,"end_line":246,"end_character":24},"updated":"2022-09-16 08:04:06.000000000","message":"no_suffix?","commit_id":"8bfa800e4d87e76f8c6861deefa9229e93bf1cc0"},{"author":{"_account_id":1002042,"name":"Tim Nordell","email":"tim.nordell@nimbelink.com"},"change_message_id":"4a552bfb866fe9659a89564be7df2e4ef37444a0","unresolved":false,"context_lines":[{"line_number":243,"context_line":"\tsize_t len \u003d unhexify((uint8_t *)cur_sym, strchr(packet + 8, \u0027:\u0027) + 1, strlen(strchr(packet + 8, \u0027:\u0027) + 1));"},{"line_number":244,"context_line":"\tcur_sym[len] \u003d 0;"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"\tconst char first_suffix[] \u003d \"\";"},{"line_number":247,"context_line":"\tconst char lto_suffix[] \u003d \".lto_priv.0\";"},{"line_number":248,"context_line":"\tconst size_t lto_suffix_len \u003d strlen(lto_suffix);"},{"line_number":249,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":8,"id":"24ef688d_9103ac45","line":246,"range":{"start_line":246,"start_character":12,"end_line":246,"end_character":24},"in_reply_to":"b15f64c1_98d8dcff","updated":"2022-09-16 14:52:15.000000000","message":"Works for me.","commit_id":"8bfa800e4d87e76f8c6861deefa9229e93bf1cc0"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"382fa57bbfcb45bd5041ed2ebae2b23e88b576a9","unresolved":true,"context_lines":[{"line_number":247,"context_line":"\tconst char lto_suffix[] \u003d \".lto_priv.0\";"},{"line_number":248,"context_line":"\tconst size_t lto_suffix_len \u003d strlen(lto_suffix);"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"\tconst char *prev_suffix;"},{"line_number":251,"context_line":"\tconst char *next_suffix;"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"\t/* Detect what suffix was used during the previous symbol lookup attempt, and"}],"source_content_type":"text/x-csrc","patch_set":8,"id":"2489bc6b_0154aeec","line":250,"range":{"start_line":250,"start_character":13,"end_line":250,"end_character":24},"updated":"2022-09-16 08:04:06.000000000","message":"\u0027prev\u0027 in the variable name seems me misleading as the variable contains the suffix we are processing currently. Should be in pair with cur_sym","commit_id":"8bfa800e4d87e76f8c6861deefa9229e93bf1cc0"},{"author":{"_account_id":1002042,"name":"Tim Nordell","email":"tim.nordell@nimbelink.com"},"change_message_id":"4a552bfb866fe9659a89564be7df2e4ef37444a0","unresolved":false,"context_lines":[{"line_number":247,"context_line":"\tconst char lto_suffix[] \u003d \".lto_priv.0\";"},{"line_number":248,"context_line":"\tconst size_t lto_suffix_len \u003d strlen(lto_suffix);"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"\tconst char *prev_suffix;"},{"line_number":251,"context_line":"\tconst char *next_suffix;"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"\t/* Detect what suffix was used during the previous symbol lookup attempt, and"}],"source_content_type":"text/x-csrc","patch_set":8,"id":"41c38704_b70d3753","line":250,"range":{"start_line":250,"start_character":13,"end_line":250,"end_character":24},"in_reply_to":"2489bc6b_0154aeec","updated":"2022-09-16 14:52:15.000000000","message":"Done.","commit_id":"8bfa800e4d87e76f8c6861deefa9229e93bf1cc0"}]}