)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"100b1dccfee00e64087d7a75b05c954a163702c2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"0d666c10_8fdf276c","updated":"2023-01-02 14:44:26.000000000","message":"Marc, would you agree on this?","commit_id":"2225b9c1ac139f9b2a580816a3df679499293cc3"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"45703de60c20e7d7435d5816523d234e79a39290","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"e96be0cb_8e2fb876","updated":"2023-01-02 13:51:50.000000000","message":"Thanks for the patch. It\u0027s a nice improvement but I can still see some corner case.\nThe ID string is user-defined and the code doesn\u0027t considers pattern repetition in ID string and before.\nE.g. imagine ID\u003d\"aaaaaaMyID\" and memory contains \"aaaa\" at the end of the 1024 byte buffer and additional \"aaaaMyID\" at the beginning of next 1024 byte buffer chunk. The memory content is thus extra \"aa\" followed by ID. The code starts counting from the first \"a\", jumps to next 1024 byte buffer chunk but has no fallback to restart the matching at second and third \"a\". Control block not found!\n\nI think a clean and readable solution consists in:\n- reduce the loop to: while (i \u003c buf_size - strlen(id)), and search only for the whole string id;\n- at end of current buffer, move the last \u0027strlen(id)-1\u0027 bytes at the beginning of the buffer, read the remaining bytes are repeat the full string search;\n- use memcmp(addr, id, strlen(id)), easier to understand.\n\nThis has the drawback to read less than 1024 bytes at each iteration after the first.\nI don\u0027t think it\u0027s a real problem but, if you want to stay in power_of_two_read_size, then need to handle both a longer buf[1024+RTT_CB_MAX_ID_LENGTH] and some flag that reports if the buffer has some previous fragment at the beginning.\n","commit_id":"2225b9c1ac139f9b2a580816a3df679499293cc3"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"100b1dccfee00e64087d7a75b05c954a163702c2","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"08f005af_7c38d830","in_reply_to":"2ab5090f_5f3a7b47","updated":"2023-01-02 14:44:26.000000000","message":"\u003e The main goal was to fix `SEGGER RTT` handling, which is the default string used by Segger MCU library\n\nI never used RTT, but agree that covering the default ID should be a must!\n\n\u003e Is handling pattern repetitions within ID string a \"must have\"?\n\nWe are in code freeze. This patch fixes a real issues and should be merged as is.\nHandling better the generic ID string is good to have, but eventually in a following patch to be handled in next dev cycle.\n\n\u003e I generally agree it would be valuable, but this involves further reimplementation of this \"find ID\" routine and makes it less performant due to full string check (though performance isn\u0027t that much of a problem for openocd in this case, right?).\n\nThe overall performance here is limited by the target_read_buffer() that goes through USB and JTAG/SWD. The computation on PC side is not really a problem.","commit_id":"2225b9c1ac139f9b2a580816a3df679499293cc3"},{"author":{"_account_id":1002107,"name":"Marcin Niestroj","email":"m.niestroj@emb.dev","username":"mniestroj"},"change_message_id":"e433caf546ff292f7883f116b284bfbb0b3351af","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2ab5090f_5f3a7b47","in_reply_to":"e96be0cb_8e2fb876","updated":"2023-01-02 14:14:12.000000000","message":"You are right about this code not being able to handle repetitions within ID string. I have spotted the same problem, though decided to leave this case as \"not handled / not supported\" for now. The main goal was to fix `SEGGER RTT` handling, which is the default string used by Segger MCU library and the string that is actually hardcoded in pyOCD implementation of RTT.\n\nIs handling pattern repetitions within ID string a \"must have\"? I generally agree it would be valuable, but this involves further reimplementation of this \"find ID\" routine and makes it less performant due to full string check (though performance isn\u0027t that much of a problem for openocd in this case, right?).","commit_id":"2225b9c1ac139f9b2a580816a3df679499293cc3"}]}