)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"143724b2342bef483a9ed8fe3faba8b95ecad5df","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"5f6d63ca_bc8742db","updated":"2024-02-04 22:06:06.000000000","message":"I\u0027m ok with the patch, but I\u0027m shocked by the huge amount of assert() that are present and the additional you have added.\nAre they really needed? Is there some issue in the framework that can make it returning an incorrect pointer without returning an error?","commit_id":"fac14c7a944b2e13e49c5b6f248e1b2605039d16"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"14123dd81f28ee9b8c2e5483a22606fa8bfea79e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"8ea123e5_b7467766","in_reply_to":"5f6d63ca_bc8742db","updated":"2024-02-20 06:04:12.000000000","message":"Agree, I had the same feeling when I was writing the code.\nThere is no suspected issue, I just wanted some safety for the new code development.\nIn practice all frequently used OpenOCD hosts uses OS with MMU and will reliably fail on a NULL ptr dereference (perhaps except OpenOCD on ESP32 IDF). So the only value added by assert on pointer is that you see immediately where the problem happened without inspecting the dumped core or replicating a problem under gdb.\nI will drop them in a new patch at the end of series.","commit_id":"fac14c7a944b2e13e49c5b6f248e1b2605039d16"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"84312b6052c36bbb2666fa4cd2e1c19141f72541","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"13b538ff_1538eaf2","updated":"2024-03-09 13:25:14.000000000","message":"Tomas,\ncan you please have a look at the clang report?","commit_id":"2db325f5395fa24aefbf137584e5fdeedf390e97"}],"src/flash/nor/nrf5.c":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"84312b6052c36bbb2666fa4cd2e1c19141f72541","unresolved":true,"context_lines":[{"line_number":1099,"context_line":"\t}"},{"line_number":1100,"context_line":""},{"line_number":1101,"context_line":"\t/* For each sector to be erased */"},{"line_number":1102,"context_line":"\tfor (unsigned int s \u003d first; s \u003c\u003d last \u0026\u0026 res \u003d\u003d ERROR_OK; s++) {"},{"line_number":1103,"context_line":""},{"line_number":1104,"context_line":"\t\tif (chip-\u003efeatures \u0026 NRF5_FEATURE_SERIES_51"},{"line_number":1105,"context_line":"\t\t\t\t\u0026\u0026 bank-\u003esectors[s].is_protected \u003d\u003d 1) {"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"bf918dab_7996671c","line":1102,"updated":"2024-03-09 13:25:14.000000000","message":"This patch triggers here a clang error:\n```The left operand of \u0027\u003d\u003d\u0027 is a garbage value```\nbecause after the change above in this function ```res``` can be not initialized.\nSee:\nhttps://build.openocd.org/job/openocd-clang/1215/clang/new/fileName.1809441744/source.588d0948-9e92-42ee-81b7-940322d9a5d8/#1074\n\nEven before this patch the check on ```res``` here was completely useless because either before the loop and inside the loop we ```return``` when ```res``` is not ```ERROR_OK```.\nWhat about dropping the check on ```res```?\n```for (unsigned int s \u003d first; s \u003c\u003d last; s++) {```\nand maybe also use ```res``` locally inside and outside the loop:\n``` int res \u003d ...``` ?","commit_id":"2db325f5395fa24aefbf137584e5fdeedf390e97"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"9f2b1fd7568acf5fa85d291b267c8dd25a40d857","unresolved":false,"context_lines":[{"line_number":1099,"context_line":"\t}"},{"line_number":1100,"context_line":""},{"line_number":1101,"context_line":"\t/* For each sector to be erased */"},{"line_number":1102,"context_line":"\tfor (unsigned int s \u003d first; s \u003c\u003d last \u0026\u0026 res \u003d\u003d ERROR_OK; s++) {"},{"line_number":1103,"context_line":""},{"line_number":1104,"context_line":"\t\tif (chip-\u003efeatures \u0026 NRF5_FEATURE_SERIES_51"},{"line_number":1105,"context_line":"\t\t\t\t\u0026\u0026 bank-\u003esectors[s].is_protected \u003d\u003d 1) {"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"0e23bd8d_f069a1bd","line":1102,"in_reply_to":"2ff325bd_6b693e56","updated":"2024-03-10 20:58:54.000000000","message":"thanks!","commit_id":"2db325f5395fa24aefbf137584e5fdeedf390e97"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"33da8f81e6a49390847906b10f34273f71e469d6","unresolved":false,"context_lines":[{"line_number":1099,"context_line":"\t}"},{"line_number":1100,"context_line":""},{"line_number":1101,"context_line":"\t/* For each sector to be erased */"},{"line_number":1102,"context_line":"\tfor (unsigned int s \u003d first; s \u003c\u003d last \u0026\u0026 res \u003d\u003d ERROR_OK; s++) {"},{"line_number":1103,"context_line":""},{"line_number":1104,"context_line":"\t\tif (chip-\u003efeatures \u0026 NRF5_FEATURE_SERIES_51"},{"line_number":1105,"context_line":"\t\t\t\t\u0026\u0026 bank-\u003esectors[s].is_protected \u003d\u003d 1) {"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"2ff325bd_6b693e56","line":1102,"in_reply_to":"bf918dab_7996671c","updated":"2024-03-10 11:42:36.000000000","message":"Sure. I missed the unusual check in\n5522: flash/nor/nrf5: check protection before flash erase/write on nRF51 | https://review.openocd.org/c/openocd/+/5522\n\nI submitted\n8174: flash/nor/nrf5: drop useless for cycle condition | https://review.openocd.org/c/openocd/+/8174","commit_id":"2db325f5395fa24aefbf137584e5fdeedf390e97"}]}
