)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"ad5db0f186925f396421d00f31a94c04361dbbda","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"61f9d764_3a1c0d46","updated":"2023-06-16 07:07:17.000000000","message":"Generally, I believe it is a good idea to add the return value to the BW/WP related functions. \n\nHowever, I am afraid there are multiple issues with this merge request. Please take a look at my comments.\n\nFor my personal curiosity - what are you using the return values for? In this merge request, they are still unused. Do you for example intend to submit another merge request that will utilize the return values somehow?","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"040db8003947e75f28b5a7b2f49967b38e78d8bb","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"f4ac64f7_12f0a977","in_reply_to":"61f9d764_3a1c0d46","updated":"2023-06-16 07:59:36.000000000","message":"I use this values in COMMAND_HANDLER for rbp and rwp commands. It gives an opportunity to show some error mesages (now user don\u0027t get any info about failure during removing bp/wp)","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"95182593642e5e82c9248ff8b5749f1120a6919a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6e3c70d8_48db6ca2","in_reply_to":"f4ac64f7_12f0a977","updated":"2023-06-19 10:36:42.000000000","message":"Thank you for explanation.","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"b83b01077305fafd77842c40fc2c85715daf6d54","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"135cdb9f_f9053f83","updated":"2023-08-07 06:26:01.000000000","message":"I\u0027m sorry for responding with a delay.\n\nTo summarize: In breakpoint_remove_all(), I\u0027d recommend to keep the original logic and continue deleting breakpoints even if an error occurs in the middle of the process (unless there is a strong reason to change the logic).\n\nThe rest of my comments are just minor remarks considering the style, comments and prints.","commit_id":"55ee148bc07719913881cd5650263840b57fae7b"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"434553f403f3d95e645735fcc798d8ee90b4dd1f","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":8,"id":"6963084b_b149bbdc","updated":"2023-08-07 10:32:58.000000000","message":"I checked the code visually. Overall, it looks fine.\n\nIt seems there are several outdated threads, not marked as \"resolved\". Could you please check if they can be closed. \n\nThanks.","commit_id":"60b37727b4ccac06d85bfef604a7783d3deb8dc8"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"bfe3e677480274d99e0d3ab9800b1bbd522f9c95","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"c774da81_e2dec4cd","in_reply_to":"3f918515_671cfc54","updated":"2023-08-07 14:38:27.000000000","message":"Done","commit_id":"60b37727b4ccac06d85bfef604a7783d3deb8dc8"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"bc2626719ecb0480131be0b5149c5f481ee0c37f","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":8,"id":"3f918515_671cfc54","in_reply_to":"6963084b_b149bbdc","updated":"2023-08-07 13:41:07.000000000","message":"All threads are resolved","commit_id":"60b37727b4ccac06d85bfef604a7783d3deb8dc8"},{"author":{"_account_id":1000853,"name":"zapb","display_name":"Marc Schink","email":"dev@zapb.de","username":"zapb"},"change_message_id":"e0067de7fefae6dde85ee7e0c09b4ab258e1c1c0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"c7c33b70_3f915f15","updated":"2023-08-13 19:42:31.000000000","message":"Looks fine for me as well. Please change the Git commit message, the first line should be short and only a single sentence.","commit_id":"36e8795abdb3cebe2ef9f47e24b16f2959b31dd3"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"74b17734b210349415d5d89fc269ca6c59120985","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"3b95b1dc_40efba7b","updated":"2023-08-11 05:46:37.000000000","message":"Thank you for this change. I don\u0027t have any further comments. It looks all right to me.","commit_id":"36e8795abdb3cebe2ef9f47e24b16f2959b31dd3"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"bf733a1328fc448369ad6c1ab3f66d5666ba0416","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"2592d0b8_4f05b7cd","in_reply_to":"c7c33b70_3f915f15","updated":"2023-08-14 10:02:57.000000000","message":"Commit message changed. Could you please mark all threads as resolved?","commit_id":"36e8795abdb3cebe2ef9f47e24b16f2959b31dd3"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"44a6999ce86c320c141ada0ea54a2ad3e9eaebc5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"308b431e_f9c06bea","updated":"2023-08-24 12:14:24.000000000","message":"Antonio, could you please take a look here?","commit_id":"48e197911357e77a7167290b6e84210684c989f9"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"072d5d7f68f60c7e6daa064d9a647ff795c4bad1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"783e0acc_2133401a","updated":"2023-08-26 13:39:45.000000000","message":"Thanks","commit_id":"48e197911357e77a7167290b6e84210684c989f9"}],"src/target/breakpoints.c":[{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"ad5db0f186925f396421d00f31a94c04361dbbda","unresolved":true,"context_lines":[{"line_number":301,"context_line":"\treturn ERROR_OK;"},{"line_number":302,"context_line":"}"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"static int breakpoint_remove_internal(struct target *target, target_addr_t address, int *status)"},{"line_number":305,"context_line":"{"},{"line_number":306,"context_line":"\tstruct breakpoint *breakpoint \u003d target-\u003ebreakpoints;"},{"line_number":307,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"2b9896d7_004defbd","line":304,"updated":"2023-06-16 07:07:17.000000000","message":"Why to add an extra argument for the return value? Can we instead return it directly?","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"040db8003947e75f28b5a7b2f49967b38e78d8bb","unresolved":true,"context_lines":[{"line_number":301,"context_line":"\treturn ERROR_OK;"},{"line_number":302,"context_line":"}"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"static int breakpoint_remove_internal(struct target *target, target_addr_t address, int *status)"},{"line_number":305,"context_line":"{"},{"line_number":306,"context_line":"\tstruct breakpoint *breakpoint \u003d target-\u003ebreakpoints;"},{"line_number":307,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"9ae97b40_08b1d411","line":304,"in_reply_to":"2b9896d7_004defbd","updated":"2023-06-16 07:59:36.000000000","message":"In breakpoint_remove func we count number of breakpoints found at address for smp target, so we need to get two values from breakpoint_remove_internal. The first value tells if there is a breakpoint at this address, the second informs about status of removing breakpoint (success/no success).","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"03441aa1fef93ffdba594a48fd64c352fbd9e656","unresolved":true,"context_lines":[{"line_number":301,"context_line":"\treturn ERROR_OK;"},{"line_number":302,"context_line":"}"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"static int breakpoint_remove_internal(struct target *target, target_addr_t address, int *status)"},{"line_number":305,"context_line":"{"},{"line_number":306,"context_line":"\tstruct breakpoint *breakpoint \u003d target-\u003ebreakpoints;"},{"line_number":307,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"b723b8c1_edb3b8ff","line":304,"in_reply_to":"9ae97b40_08b1d411","updated":"2023-06-19 10:40:15.000000000","message":"As discussed in https://review.openocd.org/c/openocd/+/7738/comment/c1f0c2c7_4e892994/, this can be accomplished by creating another ERROR_XYZ code for the case when the breakpoint is not found.","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"612bb87e31c01b275f687b38d066b3157c973d9a","unresolved":false,"context_lines":[{"line_number":301,"context_line":"\treturn ERROR_OK;"},{"line_number":302,"context_line":"}"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"static int breakpoint_remove_internal(struct target *target, target_addr_t address, int *status)"},{"line_number":305,"context_line":"{"},{"line_number":306,"context_line":"\tstruct breakpoint *breakpoint \u003d target-\u003ebreakpoints;"},{"line_number":307,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"ff35aa17_b5b5d602","line":304,"in_reply_to":"9bfa6aab_de8976b4","updated":"2023-08-07 13:35:08.000000000","message":"Done","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"b83b01077305fafd77842c40fc2c85715daf6d54","unresolved":true,"context_lines":[{"line_number":301,"context_line":"\treturn ERROR_OK;"},{"line_number":302,"context_line":"}"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"static int breakpoint_remove_internal(struct target *target, target_addr_t address, int *status)"},{"line_number":305,"context_line":"{"},{"line_number":306,"context_line":"\tstruct breakpoint *breakpoint \u003d target-\u003ebreakpoints;"},{"line_number":307,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"9bfa6aab_de8976b4","line":304,"in_reply_to":"b723b8c1_edb3b8ff","updated":"2023-08-07 06:26:01.000000000","message":"It looks like this comment has been addressed. If so, please you can mark it as resolved.","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"ad5db0f186925f396421d00f31a94c04361dbbda","unresolved":true,"context_lines":[{"line_number":318,"context_line":"\tif (breakpoint) {"},{"line_number":319,"context_line":"\t\tint retval \u003d breakpoint_free(target, breakpoint);"},{"line_number":320,"context_line":"\t\tif (retval !\u003d ERROR_OK) {"},{"line_number":321,"context_line":"\t\t\tLOG_TARGET_ERROR(target, \"fail during remove breakpoint %d\", breakpoint-\u003enumber);"},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"\t\t\tif (status)"},{"line_number":324,"context_line":"\t\t\t\t*status \u003d retval;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"571939a0_227bc522","line":321,"updated":"2023-06-16 07:07:17.000000000","message":"This seems like a duplicate error message. \n\nOne error print is in breakpoint_free(), the other one is here.\n\nCan we print the error just once?","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"040db8003947e75f28b5a7b2f49967b38e78d8bb","unresolved":true,"context_lines":[{"line_number":318,"context_line":"\tif (breakpoint) {"},{"line_number":319,"context_line":"\t\tint retval \u003d breakpoint_free(target, breakpoint);"},{"line_number":320,"context_line":"\t\tif (retval !\u003d ERROR_OK) {"},{"line_number":321,"context_line":"\t\t\tLOG_TARGET_ERROR(target, \"fail during remove breakpoint %d\", breakpoint-\u003enumber);"},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"\t\t\tif (status)"},{"line_number":324,"context_line":"\t\t\t\t*status \u003d retval;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"cc75ca5c_74016ab0","line":321,"in_reply_to":"571939a0_227bc522","updated":"2023-06-16 07:59:36.000000000","message":"Duplicate from breakpoint_remove_internal() deleted, addressed","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"95182593642e5e82c9248ff8b5749f1120a6919a","unresolved":false,"context_lines":[{"line_number":318,"context_line":"\tif (breakpoint) {"},{"line_number":319,"context_line":"\t\tint retval \u003d breakpoint_free(target, breakpoint);"},{"line_number":320,"context_line":"\t\tif (retval !\u003d ERROR_OK) {"},{"line_number":321,"context_line":"\t\t\tLOG_TARGET_ERROR(target, \"fail during remove breakpoint %d\", breakpoint-\u003enumber);"},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"\t\t\tif (status)"},{"line_number":324,"context_line":"\t\t\t\t*status \u003d retval;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"297b7085_3cdbb919","line":321,"in_reply_to":"cc75ca5c_74016ab0","updated":"2023-06-19 10:36:42.000000000","message":"Done","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"ad5db0f186925f396421d00f31a94c04361dbbda","unresolved":true,"context_lines":[{"line_number":324,"context_line":"\t\t\t\t*status \u003d retval;"},{"line_number":325,"context_line":"\t\t}"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"\t\treturn 1;"},{"line_number":328,"context_line":"\t} else {"},{"line_number":329,"context_line":"\t\tif (!target-\u003esmp)"},{"line_number":330,"context_line":"\t\t\tLOG_ERROR(\"no breakpoint at address \" TARGET_ADDR_FMT \" found\", address);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"c1f0c2c7_4e892994","line":327,"updated":"2023-06-16 07:07:17.000000000","message":"While making changes here, it\u0027d be good to replace `return 0` with `return ERROR_OK`, etc.","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"95182593642e5e82c9248ff8b5749f1120a6919a","unresolved":true,"context_lines":[{"line_number":324,"context_line":"\t\t\t\t*status \u003d retval;"},{"line_number":325,"context_line":"\t\t}"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"\t\treturn 1;"},{"line_number":328,"context_line":"\t} else {"},{"line_number":329,"context_line":"\t\tif (!target-\u003esmp)"},{"line_number":330,"context_line":"\t\t\tLOG_ERROR(\"no breakpoint at address \" TARGET_ADDR_FMT \" found\", address);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"ad28b246_2111b50c","line":327,"in_reply_to":"0cc1aebb_e9500160","updated":"2023-06-19 10:36:42.000000000","message":"I think there are two options, actually:\n\na) Return ERROR_XYZ and add the extra parameter \"found\", as Antonio suggests above.\n\nb) Define another error code, e.g. ERROR_BREAKPOINT_NOT_FOUND, which would signalize the situation that there is no such breakpoint.\n\nBoth options seem acceptable to me, but I\u0027m slightly more inclined to b) which I believe is cleaner. The reason is that not finding the breakpoint is in this case always an error as such. So the nature of the error should be signalled by the returned status (without any extra parameter). As a bonus, having less function parameters makes the functions a bit easier to use.","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"b83b01077305fafd77842c40fc2c85715daf6d54","unresolved":true,"context_lines":[{"line_number":324,"context_line":"\t\t\t\t*status \u003d retval;"},{"line_number":325,"context_line":"\t\t}"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"\t\treturn 1;"},{"line_number":328,"context_line":"\t} else {"},{"line_number":329,"context_line":"\t\tif (!target-\u003esmp)"},{"line_number":330,"context_line":"\t\t\tLOG_ERROR(\"no breakpoint at address \" TARGET_ADDR_FMT \" found\", address);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"e4d58925_1947cb76","line":327,"in_reply_to":"98781fc6_693e6b45","updated":"2023-08-07 06:26:01.000000000","message":"It looks like this comment has been addressed. If so, please you can mark it as resolved.","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"d7eb48c16f4f3888b5f720465740345f5f552a52","unresolved":true,"context_lines":[{"line_number":324,"context_line":"\t\t\t\t*status \u003d retval;"},{"line_number":325,"context_line":"\t\t}"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"\t\treturn 1;"},{"line_number":328,"context_line":"\t} else {"},{"line_number":329,"context_line":"\t\tif (!target-\u003esmp)"},{"line_number":330,"context_line":"\t\t\tLOG_ERROR(\"no breakpoint at address \" TARGET_ADDR_FMT \" found\", address);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"b8487d48_f8dc4eb1","line":327,"in_reply_to":"ad28b246_2111b50c","updated":"2023-06-20 10:54:06.000000000","message":"If we add ERROR_BREAKPOINT_NOT_FOUND returned value here, what should we do in breakpoint_remove() if we get this error code in case of smp target? Should we stop removing and rethrow this error code or continue and just don\u0027t increase num_watchpoints value? (Same question as https://review.openocd.org/c/openocd/+/7738/comment/f4f49245_35dab51c/, I think. Should I notify someone here or I can just wait for more reviewers? )","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"e10883a8969abbc47abe5f17769e5b47a810fab1","unresolved":true,"context_lines":[{"line_number":324,"context_line":"\t\t\t\t*status \u003d retval;"},{"line_number":325,"context_line":"\t\t}"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"\t\treturn 1;"},{"line_number":328,"context_line":"\t} else {"},{"line_number":329,"context_line":"\t\tif (!target-\u003esmp)"},{"line_number":330,"context_line":"\t\t\tLOG_ERROR(\"no breakpoint at address \" TARGET_ADDR_FMT \" found\", address);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"0cc1aebb_e9500160","line":327,"in_reply_to":"b0286a9a_75ba71b5","updated":"2023-06-18 22:27:22.000000000","message":"I think here we should return ERROR_XXX and propagate the 0/1 value (or true/false) through an extra parameter \"found\".","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1000853,"name":"zapb","display_name":"Marc Schink","email":"dev@zapb.de","username":"zapb"},"change_message_id":"f783bfa7f4121f0f0887a85ed5d204da65ee70b2","unresolved":true,"context_lines":[{"line_number":324,"context_line":"\t\t\t\t*status \u003d retval;"},{"line_number":325,"context_line":"\t\t}"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"\t\treturn 1;"},{"line_number":328,"context_line":"\t} else {"},{"line_number":329,"context_line":"\t\tif (!target-\u003esmp)"},{"line_number":330,"context_line":"\t\t\tLOG_ERROR(\"no breakpoint at address \" TARGET_ADDR_FMT \" found\", address);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"98781fc6_693e6b45","line":327,"in_reply_to":"b8487d48_f8dc4eb1","updated":"2023-07-27 11:35:17.000000000","message":"I also would prefer b) here. I have no experience with the behaviour of SMP targets at all - so I cannot comment on this issue, sorry.","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"040db8003947e75f28b5a7b2f49967b38e78d8bb","unresolved":true,"context_lines":[{"line_number":324,"context_line":"\t\t\t\t*status \u003d retval;"},{"line_number":325,"context_line":"\t\t}"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"\t\treturn 1;"},{"line_number":328,"context_line":"\t} else {"},{"line_number":329,"context_line":"\t\tif (!target-\u003esmp)"},{"line_number":330,"context_line":"\t\t\tLOG_ERROR(\"no breakpoint at address \" TARGET_ADDR_FMT \" found\", address);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"b0286a9a_75ba71b5","line":327,"in_reply_to":"c1f0c2c7_4e892994","updated":"2023-06-16 07:59:36.000000000","message":"Return 1(0) here means: 1(0) breakpoints found at address, if I understand this logic correctly","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"612bb87e31c01b275f687b38d066b3157c973d9a","unresolved":false,"context_lines":[{"line_number":324,"context_line":"\t\t\t\t*status \u003d retval;"},{"line_number":325,"context_line":"\t\t}"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"\t\treturn 1;"},{"line_number":328,"context_line":"\t} else {"},{"line_number":329,"context_line":"\t\tif (!target-\u003esmp)"},{"line_number":330,"context_line":"\t\t\tLOG_ERROR(\"no breakpoint at address \" TARGET_ADDR_FMT \" found\", address);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"c00ec191_c81a02b2","line":327,"in_reply_to":"e4d58925_1947cb76","updated":"2023-08-07 13:35:08.000000000","message":"Done","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"ad5db0f186925f396421d00f31a94c04361dbbda","unresolved":true,"context_lines":[{"line_number":341,"context_line":"\t\tbreakpoint \u003d breakpoint-\u003enext;"},{"line_number":342,"context_line":"\t\tint retval \u003d breakpoint_free(target, tmp);"},{"line_number":343,"context_line":"\t\tif (retval !\u003d ERROR_OK) {"},{"line_number":344,"context_line":"\t\t\tLOG_TARGET_ERROR(target, \"fail during remove breakpoint %d\", tmp-\u003enumber);"},{"line_number":345,"context_line":"\t\t\treturn retval;"},{"line_number":346,"context_line":"\t\t}"},{"line_number":347,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"54314d7f_c7e51176","line":344,"updated":"2023-06-16 07:07:17.000000000","message":"Same as above - this looks like a duplicate error print.","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"040db8003947e75f28b5a7b2f49967b38e78d8bb","unresolved":true,"context_lines":[{"line_number":341,"context_line":"\t\tbreakpoint \u003d breakpoint-\u003enext;"},{"line_number":342,"context_line":"\t\tint retval \u003d breakpoint_free(target, tmp);"},{"line_number":343,"context_line":"\t\tif (retval !\u003d ERROR_OK) {"},{"line_number":344,"context_line":"\t\t\tLOG_TARGET_ERROR(target, \"fail during remove breakpoint %d\", tmp-\u003enumber);"},{"line_number":345,"context_line":"\t\t\treturn retval;"},{"line_number":346,"context_line":"\t\t}"},{"line_number":347,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"c0161551_a10d5e17","line":344,"in_reply_to":"54314d7f_c7e51176","updated":"2023-06-16 07:59:36.000000000","message":"Duplicate deleted, addressed","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"95182593642e5e82c9248ff8b5749f1120a6919a","unresolved":false,"context_lines":[{"line_number":341,"context_line":"\t\tbreakpoint \u003d breakpoint-\u003enext;"},{"line_number":342,"context_line":"\t\tint retval \u003d breakpoint_free(target, tmp);"},{"line_number":343,"context_line":"\t\tif (retval !\u003d ERROR_OK) {"},{"line_number":344,"context_line":"\t\t\tLOG_TARGET_ERROR(target, \"fail during remove breakpoint %d\", tmp-\u003enumber);"},{"line_number":345,"context_line":"\t\t\treturn retval;"},{"line_number":346,"context_line":"\t\t}"},{"line_number":347,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"7b61bbad_5f969e24","line":344,"in_reply_to":"c0161551_a10d5e17","updated":"2023-06-19 10:36:42.000000000","message":"Done","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"ad5db0f186925f396421d00f31a94c04361dbbda","unresolved":true,"context_lines":[{"line_number":366,"context_line":"\t\t\t\treturn status;"},{"line_number":367,"context_line":"\t\t}"},{"line_number":368,"context_line":"\t\tif (num_breakpoints \u003d\u003d 0)"},{"line_number":369,"context_line":"\t\t\tLOG_ERROR(\"no breakpoint at address \" TARGET_ADDR_FMT \" found\", address);"},{"line_number":370,"context_line":"\t} else {"},{"line_number":371,"context_line":"\t\tbreakpoint_remove_internal(target, address, \u0026status);"},{"line_number":372,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"62ae6b56_797b3f3f","line":369,"updated":"2023-06-16 07:07:17.000000000","message":"While making changes in these functions, it\u0027d be good to utilize `LOG_TARGET_ERROR` (and friends) where possible.","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"85745d9fadd540abcce943b96a48610bf3aba49a","unresolved":false,"context_lines":[{"line_number":366,"context_line":"\t\t\t\treturn status;"},{"line_number":367,"context_line":"\t\t}"},{"line_number":368,"context_line":"\t\tif (num_breakpoints \u003d\u003d 0)"},{"line_number":369,"context_line":"\t\t\tLOG_ERROR(\"no breakpoint at address \" TARGET_ADDR_FMT \" found\", address);"},{"line_number":370,"context_line":"\t} else {"},{"line_number":371,"context_line":"\t\tbreakpoint_remove_internal(target, address, \u0026status);"},{"line_number":372,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"af7b4cd5_74c0efad","line":369,"in_reply_to":"2cdb0638_03c69c6b","updated":"2023-06-22 07:34:02.000000000","message":"Done","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"e10883a8969abbc47abe5f17769e5b47a810fab1","unresolved":true,"context_lines":[{"line_number":366,"context_line":"\t\t\t\treturn status;"},{"line_number":367,"context_line":"\t\t}"},{"line_number":368,"context_line":"\t\tif (num_breakpoints \u003d\u003d 0)"},{"line_number":369,"context_line":"\t\t\tLOG_ERROR(\"no breakpoint at address \" TARGET_ADDR_FMT \" found\", address);"},{"line_number":370,"context_line":"\t} else {"},{"line_number":371,"context_line":"\t\tbreakpoint_remove_internal(target, address, \u0026status);"},{"line_number":372,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"ed50ca72_73403be7","line":369,"in_reply_to":"46f05e1f_23b49d3d","updated":"2023-06-18 22:27:22.000000000","message":"I think it\u0027s still good to have.\nI work with a SOC that has 2 Cortex-A in SMP and one Cortex-M. Having the name of the target that fails let me detect if it\u0027s in the SMP or in the MCU.\nMy concern is, instead, if this should belong to this same patch or to a separate one.","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"040db8003947e75f28b5a7b2f49967b38e78d8bb","unresolved":true,"context_lines":[{"line_number":366,"context_line":"\t\t\t\treturn status;"},{"line_number":367,"context_line":"\t\t}"},{"line_number":368,"context_line":"\t\tif (num_breakpoints \u003d\u003d 0)"},{"line_number":369,"context_line":"\t\t\tLOG_ERROR(\"no breakpoint at address \" TARGET_ADDR_FMT \" found\", address);"},{"line_number":370,"context_line":"\t} else {"},{"line_number":371,"context_line":"\t\tbreakpoint_remove_internal(target, address, \u0026status);"},{"line_number":372,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"46f05e1f_23b49d3d","line":369,"in_reply_to":"62ae6b56_797b3f3f","updated":"2023-06-16 07:59:36.000000000","message":"This error message will printed only if we trying to remove breakpoint in smp target. It\u0027s strange for me to use LOG_TARGET_ERROR here","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"0749b18bb25997c9cd6439a2897c776380af15d5","unresolved":true,"context_lines":[{"line_number":366,"context_line":"\t\t\t\treturn status;"},{"line_number":367,"context_line":"\t\t}"},{"line_number":368,"context_line":"\t\tif (num_breakpoints \u003d\u003d 0)"},{"line_number":369,"context_line":"\t\t\tLOG_ERROR(\"no breakpoint at address \" TARGET_ADDR_FMT \" found\", address);"},{"line_number":370,"context_line":"\t} else {"},{"line_number":371,"context_line":"\t\tbreakpoint_remove_internal(target, address, \u0026status);"},{"line_number":372,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"2cdb0638_03c69c6b","line":369,"in_reply_to":"ddc115da_e3928967","updated":"2023-06-20 10:56:52.000000000","message":"So, can it be marked as resolved?","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"95182593642e5e82c9248ff8b5749f1120a6919a","unresolved":true,"context_lines":[{"line_number":366,"context_line":"\t\t\t\treturn status;"},{"line_number":367,"context_line":"\t\t}"},{"line_number":368,"context_line":"\t\tif (num_breakpoints \u003d\u003d 0)"},{"line_number":369,"context_line":"\t\t\tLOG_ERROR(\"no breakpoint at address \" TARGET_ADDR_FMT \" found\", address);"},{"line_number":370,"context_line":"\t} else {"},{"line_number":371,"context_line":"\t\tbreakpoint_remove_internal(target, address, \u0026status);"},{"line_number":372,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"ddc115da_e3928967","line":369,"in_reply_to":"ed50ca72_73403be7","updated":"2023-06-19 10:36:42.000000000","message":"Hi Antonio, I think you\u0027re right, this can be done in a separate patch.","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"ad5db0f186925f396421d00f31a94c04361dbbda","unresolved":true,"context_lines":[{"line_number":538,"context_line":"\tif (retval !\u003d ERROR_OK) {"},{"line_number":539,"context_line":"\t\tLOG_TARGET_ERROR(target, \"fail during running target-specific part of removing watchpoint %d\","},{"line_number":540,"context_line":"\t\t\t\t\t\twatchpoint-\u003enumber);"},{"line_number":541,"context_line":"\t\treturn retval;"},{"line_number":542,"context_line":"\t}"},{"line_number":543,"context_line":""},{"line_number":544,"context_line":"\tLOG_DEBUG(\"free WPID: %d --\u003e %d\", watchpoint-\u003eunique_id, retval);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"9e4fa784_9e377a82","line":541,"updated":"2023-06-16 07:07:17.000000000","message":"This changes the semantics of the function: \n\nWhen the breakpoint removal fails, you skip the `free(watchpoint)`, which seems not correct thing to do.\n\nI believe we should free the watchpoint structure in any case, as the original code did it.","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"b83b01077305fafd77842c40fc2c85715daf6d54","unresolved":false,"context_lines":[{"line_number":538,"context_line":"\tif (retval !\u003d ERROR_OK) {"},{"line_number":539,"context_line":"\t\tLOG_TARGET_ERROR(target, \"fail during running target-specific part of removing watchpoint %d\","},{"line_number":540,"context_line":"\t\t\t\t\t\twatchpoint-\u003enumber);"},{"line_number":541,"context_line":"\t\treturn retval;"},{"line_number":542,"context_line":"\t}"},{"line_number":543,"context_line":""},{"line_number":544,"context_line":"\tLOG_DEBUG(\"free WPID: %d --\u003e %d\", watchpoint-\u003eunique_id, retval);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"d4a381e8_88657899","line":541,"in_reply_to":"7083409e_d1534e3e","updated":"2023-08-07 06:26:01.000000000","message":"After taking a second look at this one, I believe the proposed implementation is fine. Marking this comment as resolved.","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"040db8003947e75f28b5a7b2f49967b38e78d8bb","unresolved":true,"context_lines":[{"line_number":538,"context_line":"\tif (retval !\u003d ERROR_OK) {"},{"line_number":539,"context_line":"\t\tLOG_TARGET_ERROR(target, \"fail during running target-specific part of removing watchpoint %d\","},{"line_number":540,"context_line":"\t\t\t\t\t\twatchpoint-\u003enumber);"},{"line_number":541,"context_line":"\t\treturn retval;"},{"line_number":542,"context_line":"\t}"},{"line_number":543,"context_line":""},{"line_number":544,"context_line":"\tLOG_DEBUG(\"free WPID: %d --\u003e %d\", watchpoint-\u003eunique_id, retval);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"c18d61b1_efecebb6","line":541,"in_reply_to":"9e4fa784_9e377a82","updated":"2023-06-16 07:59:36.000000000","message":"Main idea of this pr is change this semantic, because as for me it isn\u0027t correct. I faced with such a problem: if we try to remove watchpoint on running target, we don\u0027t get any error messages (only in a log file) and we lost internal watchpoint, but hardware trigger will remain on place. In my opinion, it\u0027s incorrect behaviour","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"95182593642e5e82c9248ff8b5749f1120a6919a","unresolved":true,"context_lines":[{"line_number":538,"context_line":"\tif (retval !\u003d ERROR_OK) {"},{"line_number":539,"context_line":"\t\tLOG_TARGET_ERROR(target, \"fail during running target-specific part of removing watchpoint %d\","},{"line_number":540,"context_line":"\t\t\t\t\t\twatchpoint-\u003enumber);"},{"line_number":541,"context_line":"\t\treturn retval;"},{"line_number":542,"context_line":"\t}"},{"line_number":543,"context_line":""},{"line_number":544,"context_line":"\tLOG_DEBUG(\"free WPID: %d --\u003e %d\", watchpoint-\u003eunique_id, retval);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"7083409e_d1534e3e","line":541,"in_reply_to":"c18d61b1_efecebb6","updated":"2023-06-19 10:36:42.000000000","message":"Thanks, that explanation looks all right to me. Still, I\u0027d recommend that someone else takes a look at this specific case and confirms.","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"ad5db0f186925f396421d00f31a94c04361dbbda","unresolved":true,"context_lines":[{"line_number":548,"context_line":"\treturn ERROR_OK;"},{"line_number":549,"context_line":"}"},{"line_number":550,"context_line":""},{"line_number":551,"context_line":"static int watchpoint_remove_internal(struct target *target, target_addr_t address, int *status)"},{"line_number":552,"context_line":"{"},{"line_number":553,"context_line":"\tstruct watchpoint *watchpoint \u003d target-\u003ewatchpoints;"},{"line_number":554,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"d9d46738_14c5dfff","line":551,"updated":"2023-06-16 07:07:17.000000000","message":"Same as above - please don\u0027t add extra status arguments. Instead, use the `int` return value that\u0027s already there.","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"95182593642e5e82c9248ff8b5749f1120a6919a","unresolved":false,"context_lines":[{"line_number":548,"context_line":"\treturn ERROR_OK;"},{"line_number":549,"context_line":"}"},{"line_number":550,"context_line":""},{"line_number":551,"context_line":"static int watchpoint_remove_internal(struct target *target, target_addr_t address, int *status)"},{"line_number":552,"context_line":"{"},{"line_number":553,"context_line":"\tstruct watchpoint *watchpoint \u003d target-\u003ewatchpoints;"},{"line_number":554,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"18cf8de6_3e5e60b5","line":551,"in_reply_to":"c8c3dfbb_b9a3d847","updated":"2023-06-19 10:36:42.000000000","message":"I\u0027m marking this thread as \"Done\" since the discussion already continues above (https://review.openocd.org/c/openocd/+/7738/comment/c1f0c2c7_4e892994/).","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"040db8003947e75f28b5a7b2f49967b38e78d8bb","unresolved":true,"context_lines":[{"line_number":548,"context_line":"\treturn ERROR_OK;"},{"line_number":549,"context_line":"}"},{"line_number":550,"context_line":""},{"line_number":551,"context_line":"static int watchpoint_remove_internal(struct target *target, target_addr_t address, int *status)"},{"line_number":552,"context_line":"{"},{"line_number":553,"context_line":"\tstruct watchpoint *watchpoint \u003d target-\u003ewatchpoints;"},{"line_number":554,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"c8c3dfbb_b9a3d847","line":551,"in_reply_to":"d9d46738_14c5dfff","updated":"2023-06-16 07:59:36.000000000","message":"Same reply as above","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"ad5db0f186925f396421d00f31a94c04361dbbda","unresolved":true,"context_lines":[{"line_number":593,"context_line":"\t\t\tif (status \u003d\u003d ERROR_OK)"},{"line_number":594,"context_line":"\t\t\t\tnum_watchpoints +\u003d retval;"},{"line_number":595,"context_line":"\t\t\telse"},{"line_number":596,"context_line":"\t\t\t\treturn status;"},{"line_number":597,"context_line":"\t\t}"},{"line_number":598,"context_line":"\t\tif (num_watchpoints \u003d\u003d 0)"},{"line_number":599,"context_line":"\t\t\tLOG_ERROR(\"no watchpoint at address \" TARGET_ADDR_FMT \" found\", address);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"f4f49245_35dab51c","line":596,"updated":"2023-06-16 07:07:17.000000000","message":"This changes the logic of the function. Do we really need to `return` from the function on the first encountered failure, and not continue with the loop?","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1000853,"name":"zapb","display_name":"Marc Schink","email":"dev@zapb.de","username":"zapb"},"change_message_id":"f783bfa7f4121f0f0887a85ed5d204da65ee70b2","unresolved":true,"context_lines":[{"line_number":593,"context_line":"\t\t\tif (status \u003d\u003d ERROR_OK)"},{"line_number":594,"context_line":"\t\t\t\tnum_watchpoints +\u003d retval;"},{"line_number":595,"context_line":"\t\t\telse"},{"line_number":596,"context_line":"\t\t\t\treturn status;"},{"line_number":597,"context_line":"\t\t}"},{"line_number":598,"context_line":"\t\tif (num_watchpoints \u003d\u003d 0)"},{"line_number":599,"context_line":"\t\t\tLOG_ERROR(\"no watchpoint at address \" TARGET_ADDR_FMT \" found\", address);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"796bd549_0e149868","line":596,"in_reply_to":"38a46af9_20d46b14","updated":"2023-07-27 11:35:17.000000000","message":"I also trend to prefer the original behaviour. If we do not have a good reason to change the original behaviour we should keep it in my opinion. But I don\u0027t have a strong opinion here.","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"d6162e1ee9d5491162e1de8f32219cf2af57d3bf","unresolved":false,"context_lines":[{"line_number":593,"context_line":"\t\t\tif (status \u003d\u003d ERROR_OK)"},{"line_number":594,"context_line":"\t\t\t\tnum_watchpoints +\u003d retval;"},{"line_number":595,"context_line":"\t\t\telse"},{"line_number":596,"context_line":"\t\t\t\treturn status;"},{"line_number":597,"context_line":"\t\t}"},{"line_number":598,"context_line":"\t\tif (num_watchpoints \u003d\u003d 0)"},{"line_number":599,"context_line":"\t\t\tLOG_ERROR(\"no watchpoint at address \" TARGET_ADDR_FMT \" found\", address);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"6d8ecab6_9feddcd5","line":596,"in_reply_to":"796bd549_0e149868","updated":"2023-08-07 13:34:15.000000000","message":"Return the original logic (continue removing in case of failure)","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"95182593642e5e82c9248ff8b5749f1120a6919a","unresolved":true,"context_lines":[{"line_number":593,"context_line":"\t\t\tif (status \u003d\u003d ERROR_OK)"},{"line_number":594,"context_line":"\t\t\t\tnum_watchpoints +\u003d retval;"},{"line_number":595,"context_line":"\t\t\telse"},{"line_number":596,"context_line":"\t\t\t\treturn status;"},{"line_number":597,"context_line":"\t\t}"},{"line_number":598,"context_line":"\t\tif (num_watchpoints \u003d\u003d 0)"},{"line_number":599,"context_line":"\t\t\tLOG_ERROR(\"no watchpoint at address \" TARGET_ADDR_FMT \" found\", address);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"38a46af9_20d46b14","line":596,"in_reply_to":"ae58eea7_5960a7e0","updated":"2023-06-19 10:36:42.000000000","message":"I tend to prefer the original behaviro - try to delete everything even if an error occurs during deletion of a certain item.\n\nAgain, I\u0027d recommend that someone else takes a look at this case to help decide whether to keep the original logic (try delete everything) or use the new one (give up on first error).","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"040db8003947e75f28b5a7b2f49967b38e78d8bb","unresolved":true,"context_lines":[{"line_number":593,"context_line":"\t\t\tif (status \u003d\u003d ERROR_OK)"},{"line_number":594,"context_line":"\t\t\t\tnum_watchpoints +\u003d retval;"},{"line_number":595,"context_line":"\t\t\telse"},{"line_number":596,"context_line":"\t\t\t\treturn status;"},{"line_number":597,"context_line":"\t\t}"},{"line_number":598,"context_line":"\t\tif (num_watchpoints \u003d\u003d 0)"},{"line_number":599,"context_line":"\t\t\tLOG_ERROR(\"no watchpoint at address \" TARGET_ADDR_FMT \" found\", address);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"ae58eea7_5960a7e0","line":596,"in_reply_to":"f4f49245_35dab51c","updated":"2023-06-16 07:59:36.000000000","message":"I think that both scenarios (mine and yours) are correct, I just chose one of them, because it seemed to me that it would be better not to try to delete further, but first look at the error and then, perhaps, restart the deletion","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"ad5db0f186925f396421d00f31a94c04361dbbda","unresolved":true,"context_lines":[{"line_number":610,"context_line":"\t\ttarget_name(target));"},{"line_number":611,"context_line":"\twhile (target-\u003ewatchpoints)"},{"line_number":612,"context_line":"\t\tif (watchpoint_free(target, target-\u003ewatchpoints) !\u003d ERROR_OK) {"},{"line_number":613,"context_line":"\t\t\tLOG_TARGET_ERROR(target, \"fail during remove breakpoint %d\", target-\u003ebreakpoints-\u003enumber);"},{"line_number":614,"context_line":"\t\t\treturn;"},{"line_number":615,"context_line":"\t\t}"},{"line_number":616,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"8093da67_6c89d368","line":613,"updated":"2023-06-16 07:07:17.000000000","message":"This seems like a duplicate error message. There is already an error message  inside watchpoint_free().\n\nFurthermore, this seems to be a copy-paste typo - we\u0027re removing watchpoints but the log message refers to breakpoints.","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"95182593642e5e82c9248ff8b5749f1120a6919a","unresolved":false,"context_lines":[{"line_number":610,"context_line":"\t\ttarget_name(target));"},{"line_number":611,"context_line":"\twhile (target-\u003ewatchpoints)"},{"line_number":612,"context_line":"\t\tif (watchpoint_free(target, target-\u003ewatchpoints) !\u003d ERROR_OK) {"},{"line_number":613,"context_line":"\t\t\tLOG_TARGET_ERROR(target, \"fail during remove breakpoint %d\", target-\u003ebreakpoints-\u003enumber);"},{"line_number":614,"context_line":"\t\t\treturn;"},{"line_number":615,"context_line":"\t\t}"},{"line_number":616,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"aadc4fca_3eaf9584","line":613,"in_reply_to":"42a259bf_9db34ac3","updated":"2023-06-19 10:36:42.000000000","message":"Done","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"040db8003947e75f28b5a7b2f49967b38e78d8bb","unresolved":true,"context_lines":[{"line_number":610,"context_line":"\t\ttarget_name(target));"},{"line_number":611,"context_line":"\twhile (target-\u003ewatchpoints)"},{"line_number":612,"context_line":"\t\tif (watchpoint_free(target, target-\u003ewatchpoints) !\u003d ERROR_OK) {"},{"line_number":613,"context_line":"\t\t\tLOG_TARGET_ERROR(target, \"fail during remove breakpoint %d\", target-\u003ebreakpoints-\u003enumber);"},{"line_number":614,"context_line":"\t\t\treturn;"},{"line_number":615,"context_line":"\t\t}"},{"line_number":616,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"42a259bf_9db34ac3","line":613,"in_reply_to":"8093da67_6c89d368","updated":"2023-06-16 07:59:36.000000000","message":"Ooops, stupid mistake.\nDuplicate deleted, addressed","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"ad5db0f186925f396421d00f31a94c04361dbbda","unresolved":true,"context_lines":[{"line_number":611,"context_line":"\twhile (target-\u003ewatchpoints)"},{"line_number":612,"context_line":"\t\tif (watchpoint_free(target, target-\u003ewatchpoints) !\u003d ERROR_OK) {"},{"line_number":613,"context_line":"\t\t\tLOG_TARGET_ERROR(target, \"fail during remove breakpoint %d\", target-\u003ebreakpoints-\u003enumber);"},{"line_number":614,"context_line":"\t\t\treturn;"},{"line_number":615,"context_line":"\t\t}"},{"line_number":616,"context_line":"}"},{"line_number":617,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"386db045_819399e9","line":614,"updated":"2023-06-16 07:07:17.000000000","message":"Why should we exist the function on first failure, and not continue the loop?","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"612bb87e31c01b275f687b38d066b3157c973d9a","unresolved":false,"context_lines":[{"line_number":611,"context_line":"\twhile (target-\u003ewatchpoints)"},{"line_number":612,"context_line":"\t\tif (watchpoint_free(target, target-\u003ewatchpoints) !\u003d ERROR_OK) {"},{"line_number":613,"context_line":"\t\t\tLOG_TARGET_ERROR(target, \"fail during remove breakpoint %d\", target-\u003ebreakpoints-\u003enumber);"},{"line_number":614,"context_line":"\t\t\treturn;"},{"line_number":615,"context_line":"\t\t}"},{"line_number":616,"context_line":"}"},{"line_number":617,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"b42c71f0_ade3a305","line":614,"in_reply_to":"103129b7_3a104d9d","updated":"2023-08-07 13:35:08.000000000","message":"Done","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"040db8003947e75f28b5a7b2f49967b38e78d8bb","unresolved":true,"context_lines":[{"line_number":611,"context_line":"\twhile (target-\u003ewatchpoints)"},{"line_number":612,"context_line":"\t\tif (watchpoint_free(target, target-\u003ewatchpoints) !\u003d ERROR_OK) {"},{"line_number":613,"context_line":"\t\t\tLOG_TARGET_ERROR(target, \"fail during remove breakpoint %d\", target-\u003ebreakpoints-\u003enumber);"},{"line_number":614,"context_line":"\t\t\treturn;"},{"line_number":615,"context_line":"\t\t}"},{"line_number":616,"context_line":"}"},{"line_number":617,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"b907b657_a5efe472","line":614,"in_reply_to":"386db045_819399e9","updated":"2023-06-16 07:59:36.000000000","message":"Same logic as in #596","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"b83b01077305fafd77842c40fc2c85715daf6d54","unresolved":true,"context_lines":[{"line_number":611,"context_line":"\twhile (target-\u003ewatchpoints)"},{"line_number":612,"context_line":"\t\tif (watchpoint_free(target, target-\u003ewatchpoints) !\u003d ERROR_OK) {"},{"line_number":613,"context_line":"\t\t\tLOG_TARGET_ERROR(target, \"fail during remove breakpoint %d\", target-\u003ebreakpoints-\u003enumber);"},{"line_number":614,"context_line":"\t\t\treturn;"},{"line_number":615,"context_line":"\t\t}"},{"line_number":616,"context_line":"}"},{"line_number":617,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"103129b7_3a104d9d","line":614,"in_reply_to":"b907b657_a5efe472","updated":"2023-08-07 06:26:01.000000000","message":"Is this one resolved? If so, please mark it that way.","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1000853,"name":"zapb","display_name":"Marc Schink","email":"dev@zapb.de","username":"zapb"},"change_message_id":"8e3a3eae8662526c65818b06812a52584a3088fe","unresolved":true,"context_lines":[{"line_number":330,"context_line":"\t\tbreakpoint \u003d breakpoint-\u003enext;"},{"line_number":331,"context_line":"\t\tint retval \u003d breakpoint_free(target, tmp);"},{"line_number":332,"context_line":"\t\tif (retval !\u003d ERROR_OK)"},{"line_number":333,"context_line":"\t\t\treturn retval;"},{"line_number":334,"context_line":"\t}"},{"line_number":335,"context_line":""},{"line_number":336,"context_line":"\treturn ERROR_OK;"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"2d744bf8_b9bceed6","line":333,"updated":"2023-08-02 19:08:33.000000000","message":"The code still changes the logic as pointed out by Jan, right?","commit_id":"a58698cefa8b0e4667cbb5ceedf736d309738305"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"2cbb7c54df10a4cf364fa9d41fc7f94225f8cb93","unresolved":true,"context_lines":[{"line_number":330,"context_line":"\t\tbreakpoint \u003d breakpoint-\u003enext;"},{"line_number":331,"context_line":"\t\tint retval \u003d breakpoint_free(target, tmp);"},{"line_number":332,"context_line":"\t\tif (retval !\u003d ERROR_OK)"},{"line_number":333,"context_line":"\t\t\treturn retval;"},{"line_number":334,"context_line":"\t}"},{"line_number":335,"context_line":""},{"line_number":336,"context_line":"\treturn ERROR_OK;"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"c6796f6d_23fb97c8","line":333,"in_reply_to":"2d744bf8_b9bceed6","updated":"2023-08-03 12:17:23.000000000","message":"Fixed","commit_id":"a58698cefa8b0e4667cbb5ceedf736d309738305"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"b83b01077305fafd77842c40fc2c85715daf6d54","unresolved":false,"context_lines":[{"line_number":330,"context_line":"\t\tbreakpoint \u003d breakpoint-\u003enext;"},{"line_number":331,"context_line":"\t\tint retval \u003d breakpoint_free(target, tmp);"},{"line_number":332,"context_line":"\t\tif (retval !\u003d ERROR_OK)"},{"line_number":333,"context_line":"\t\t\treturn retval;"},{"line_number":334,"context_line":"\t}"},{"line_number":335,"context_line":""},{"line_number":336,"context_line":"\treturn ERROR_OK;"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"c47d58ca_7252d4c3","line":333,"in_reply_to":"c6796f6d_23fb97c8","updated":"2023-08-07 06:26:01.000000000","message":"Done","commit_id":"a58698cefa8b0e4667cbb5ceedf736d309738305"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"b83b01077305fafd77842c40fc2c85715daf6d54","unresolved":true,"context_lines":[{"line_number":288,"context_line":""},{"line_number":289,"context_line":"\tretval \u003d target_remove_breakpoint(target, breakpoint);"},{"line_number":290,"context_line":"\tif (retval !\u003d ERROR_OK) {"},{"line_number":291,"context_line":"\t\tLOG_TARGET_ERROR(target, \"fail during running target-specific part of removing breakpoint %d\","},{"line_number":292,"context_line":"\t\t\t\t\t\tbreakpoint-\u003enumber);"},{"line_number":293,"context_line":"\t\treturn retval;"},{"line_number":294,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":7,"id":"6d26edbf_5c3e9d59","line":291,"updated":"2023-08-07 06:26:01.000000000","message":"If you\u0027d like, this text can be simplified to:\n\n\"could not remove breakpoint #%d on this target\"","commit_id":"55ee148bc07719913881cd5650263840b57fae7b"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"0a4538e2b1d8a4a441c6741923377f1bae8f31fe","unresolved":false,"context_lines":[{"line_number":288,"context_line":""},{"line_number":289,"context_line":"\tretval \u003d target_remove_breakpoint(target, breakpoint);"},{"line_number":290,"context_line":"\tif (retval !\u003d ERROR_OK) {"},{"line_number":291,"context_line":"\t\tLOG_TARGET_ERROR(target, \"fail during running target-specific part of removing breakpoint %d\","},{"line_number":292,"context_line":"\t\t\t\t\t\tbreakpoint-\u003enumber);"},{"line_number":293,"context_line":"\t\treturn retval;"},{"line_number":294,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":7,"id":"cf6d27ab_6f52dee4","line":291,"in_reply_to":"6d26edbf_5c3e9d59","updated":"2023-08-07 07:50:10.000000000","message":"Done","commit_id":"55ee148bc07719913881cd5650263840b57fae7b"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"b83b01077305fafd77842c40fc2c85715daf6d54","unresolved":true,"context_lines":[{"line_number":316,"context_line":"\t\treturn breakpoint_free(target, breakpoint);"},{"line_number":317,"context_line":"\t} else {"},{"line_number":318,"context_line":"\t\tif (!target-\u003esmp)"},{"line_number":319,"context_line":"\t\t\tLOG_TARGET_ERROR(target, \"no breakpoint at address \" TARGET_ADDR_FMT \" found\", address);"},{"line_number":320,"context_line":"\t\treturn ERROR_BREAKPOINT_NOT_FOUND;"},{"line_number":321,"context_line":"\t}"},{"line_number":322,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":7,"id":"9780a052_8204ea11","line":319,"updated":"2023-08-07 06:26:01.000000000","message":"If you\u0027d like, I think this word order is a bit more natural:\n\n\"no breakpoint found at address \" TARGET_ADDR_FMT","commit_id":"55ee148bc07719913881cd5650263840b57fae7b"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"0a4538e2b1d8a4a441c6741923377f1bae8f31fe","unresolved":false,"context_lines":[{"line_number":316,"context_line":"\t\treturn breakpoint_free(target, breakpoint);"},{"line_number":317,"context_line":"\t} else {"},{"line_number":318,"context_line":"\t\tif (!target-\u003esmp)"},{"line_number":319,"context_line":"\t\t\tLOG_TARGET_ERROR(target, \"no breakpoint at address \" TARGET_ADDR_FMT \" found\", address);"},{"line_number":320,"context_line":"\t\treturn ERROR_BREAKPOINT_NOT_FOUND;"},{"line_number":321,"context_line":"\t}"},{"line_number":322,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":7,"id":"6df37617_59977b0d","line":319,"in_reply_to":"9780a052_8204ea11","updated":"2023-08-07 07:50:10.000000000","message":"Done","commit_id":"55ee148bc07719913881cd5650263840b57fae7b"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"b83b01077305fafd77842c40fc2c85715daf6d54","unresolved":true,"context_lines":[{"line_number":341,"context_line":"{"},{"line_number":342,"context_line":"\tint retval \u003d ERROR_OK;"},{"line_number":343,"context_line":"\tif (target-\u003esmp) {"},{"line_number":344,"context_line":"\t\tunsigned int num_breakpoints \u003d 0;"},{"line_number":345,"context_line":"\t\tstruct target_list *head;"},{"line_number":346,"context_line":""},{"line_number":347,"context_line":"\t\tforeach_smp_target(head, target-\u003esmp_targets) {"}],"source_content_type":"text/x-csrc","patch_set":7,"id":"99b16e55_24129e42","line":344,"updated":"2023-08-07 06:26:01.000000000","message":"While making changes here, you can use this opportunity to rename this variable to:\n\nnum_removed_breakpoints","commit_id":"55ee148bc07719913881cd5650263840b57fae7b"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"0a4538e2b1d8a4a441c6741923377f1bae8f31fe","unresolved":false,"context_lines":[{"line_number":341,"context_line":"{"},{"line_number":342,"context_line":"\tint retval \u003d ERROR_OK;"},{"line_number":343,"context_line":"\tif (target-\u003esmp) {"},{"line_number":344,"context_line":"\t\tunsigned int num_breakpoints \u003d 0;"},{"line_number":345,"context_line":"\t\tstruct target_list *head;"},{"line_number":346,"context_line":""},{"line_number":347,"context_line":"\t\tforeach_smp_target(head, target-\u003esmp_targets) {"}],"source_content_type":"text/x-csrc","patch_set":7,"id":"f447fae4_568ad461","line":344,"in_reply_to":"99b16e55_24129e42","updated":"2023-08-07 07:50:10.000000000","message":"Done","commit_id":"55ee148bc07719913881cd5650263840b57fae7b"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"b83b01077305fafd77842c40fc2c85715daf6d54","unresolved":true,"context_lines":[{"line_number":356,"context_line":"\t\t\t}"},{"line_number":357,"context_line":"\t\t}"},{"line_number":358,"context_line":"\t\tif (num_breakpoints \u003d\u003d 0)"},{"line_number":359,"context_line":"\t\t\tLOG_ERROR(\"no breakpoint at address \" TARGET_ADDR_FMT \" found\", address);"},{"line_number":360,"context_line":"\t} else {"},{"line_number":361,"context_line":"\t\tretval \u003d breakpoint_remove_internal(target, address);"},{"line_number":362,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":7,"id":"bcdeb1bf_b8aa326c","line":359,"updated":"2023-08-07 06:26:01.000000000","message":"Does it make sense to use LOG_TARGET_ERROR here?\n\nThere are multiple other occurrences of plain LOG_ERROR() in this merge request. Please check if it would be better to use LOG_TARGET_ERROR() there. I won\u0027t comment on each occurrence to keep this review easy to follow.","commit_id":"55ee148bc07719913881cd5650263840b57fae7b"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"0a4538e2b1d8a4a441c6741923377f1bae8f31fe","unresolved":false,"context_lines":[{"line_number":356,"context_line":"\t\t\t}"},{"line_number":357,"context_line":"\t\t}"},{"line_number":358,"context_line":"\t\tif (num_breakpoints \u003d\u003d 0)"},{"line_number":359,"context_line":"\t\t\tLOG_ERROR(\"no breakpoint at address \" TARGET_ADDR_FMT \" found\", address);"},{"line_number":360,"context_line":"\t} else {"},{"line_number":361,"context_line":"\t\tretval \u003d breakpoint_remove_internal(target, address);"},{"line_number":362,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":7,"id":"7406cf7b_5ca0c4f0","line":359,"in_reply_to":"bcdeb1bf_b8aa326c","updated":"2023-08-07 07:50:10.000000000","message":"Done","commit_id":"55ee148bc07719913881cd5650263840b57fae7b"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"b83b01077305fafd77842c40fc2c85715daf6d54","unresolved":true,"context_lines":[{"line_number":375,"context_line":"\t\t\tretval \u003d breakpoint_remove_all_internal(curr);"},{"line_number":376,"context_line":""},{"line_number":377,"context_line":"\t\t\tif (retval !\u003d ERROR_OK)"},{"line_number":378,"context_line":"\t\t\t\treturn retval;"},{"line_number":379,"context_line":"\t\t}"},{"line_number":380,"context_line":"\t} else {"},{"line_number":381,"context_line":"\t\tretval \u003d breakpoint_remove_all_internal(target);"}],"source_content_type":"text/x-csrc","patch_set":7,"id":"cded8bbf_1cc69599","line":378,"updated":"2023-08-07 06:26:01.000000000","message":"As already discussed earlier, here I\u0027d recommend to keep deleting breakpoints from all targets even on error.\n\nIn other words, I recommend to keep the original behavior unless there is a good reason to change it.","commit_id":"55ee148bc07719913881cd5650263840b57fae7b"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"0780a466bcb744ff9ac07545d032268671adb7dd","unresolved":false,"context_lines":[{"line_number":375,"context_line":"\t\t\tretval \u003d breakpoint_remove_all_internal(curr);"},{"line_number":376,"context_line":""},{"line_number":377,"context_line":"\t\t\tif (retval !\u003d ERROR_OK)"},{"line_number":378,"context_line":"\t\t\t\treturn retval;"},{"line_number":379,"context_line":"\t\t}"},{"line_number":380,"context_line":"\t} else {"},{"line_number":381,"context_line":"\t\tretval \u003d breakpoint_remove_all_internal(target);"}],"source_content_type":"text/x-csrc","patch_set":7,"id":"ad5f8953_afa082f6","line":378,"in_reply_to":"9e866776_1fb83ef7","updated":"2023-08-07 10:30:41.000000000","message":"Done","commit_id":"55ee148bc07719913881cd5650263840b57fae7b"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"eca59359710a11731d1eb1f827a2d67ceb2cf2d6","unresolved":true,"context_lines":[{"line_number":375,"context_line":"\t\t\tretval \u003d breakpoint_remove_all_internal(curr);"},{"line_number":376,"context_line":""},{"line_number":377,"context_line":"\t\t\tif (retval !\u003d ERROR_OK)"},{"line_number":378,"context_line":"\t\t\t\treturn retval;"},{"line_number":379,"context_line":"\t\t}"},{"line_number":380,"context_line":"\t} else {"},{"line_number":381,"context_line":"\t\tretval \u003d breakpoint_remove_all_internal(target);"}],"source_content_type":"text/x-csrc","patch_set":7,"id":"9e866776_1fb83ef7","line":378,"in_reply_to":"cded8bbf_1cc69599","updated":"2023-08-07 07:58:47.000000000","message":"Addressed","commit_id":"55ee148bc07719913881cd5650263840b57fae7b"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"b83b01077305fafd77842c40fc2c85715daf6d54","unresolved":true,"context_lines":[{"line_number":573,"context_line":"{"},{"line_number":574,"context_line":"\tint retval \u003d ERROR_OK;"},{"line_number":575,"context_line":"\tif (target-\u003esmp) {"},{"line_number":576,"context_line":"\t\tunsigned int num_watchpoints \u003d 0;"},{"line_number":577,"context_line":"\t\tstruct target_list *head;"},{"line_number":578,"context_line":""},{"line_number":579,"context_line":"\t\tforeach_smp_target(head, target-\u003esmp_targets) {"}],"source_content_type":"text/x-csrc","patch_set":7,"id":"ee036b9e_0042bf3a","line":576,"updated":"2023-08-07 06:26:01.000000000","message":"This can be renamed to: num_removed_watchpoints","commit_id":"55ee148bc07719913881cd5650263840b57fae7b"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"0a4538e2b1d8a4a441c6741923377f1bae8f31fe","unresolved":false,"context_lines":[{"line_number":573,"context_line":"{"},{"line_number":574,"context_line":"\tint retval \u003d ERROR_OK;"},{"line_number":575,"context_line":"\tif (target-\u003esmp) {"},{"line_number":576,"context_line":"\t\tunsigned int num_watchpoints \u003d 0;"},{"line_number":577,"context_line":"\t\tstruct target_list *head;"},{"line_number":578,"context_line":""},{"line_number":579,"context_line":"\t\tforeach_smp_target(head, target-\u003esmp_targets) {"}],"source_content_type":"text/x-csrc","patch_set":7,"id":"f67e0051_c8960e8b","line":576,"in_reply_to":"ee036b9e_0042bf3a","updated":"2023-08-07 07:50:10.000000000","message":"Done","commit_id":"55ee148bc07719913881cd5650263840b57fae7b"},{"author":{"_account_id":1000853,"name":"zapb","display_name":"Marc Schink","email":"dev@zapb.de","username":"zapb"},"change_message_id":"f3fbe8ae69702b7078ca4466639b6f68984b9c0d","unresolved":true,"context_lines":[{"line_number":316,"context_line":"\t\treturn breakpoint_free(target, breakpoint);"},{"line_number":317,"context_line":"\t} else {"},{"line_number":318,"context_line":"\t\tif (!target-\u003esmp)"},{"line_number":319,"context_line":"\t\t\tLOG_TARGET_ERROR(target, \"no breakpoint found at address\" TARGET_ADDR_FMT, address);"},{"line_number":320,"context_line":"\t\treturn ERROR_BREAKPOINT_NOT_FOUND;"},{"line_number":321,"context_line":"\t}"},{"line_number":322,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":10,"id":"e6af2e86_5b6dad6c","line":319,"updated":"2023-08-08 21:32:03.000000000","message":"You changed the error message, why? It should at least be consistent with the other messages (see below).","commit_id":"d8001cef99716105a94896a916ca3cc280c11e9c"},{"author":{"_account_id":1000853,"name":"zapb","display_name":"Marc Schink","email":"dev@zapb.de","username":"zapb"},"change_message_id":"aed80c85e98705b6cea085547e214e0a4c39803d","unresolved":false,"context_lines":[{"line_number":316,"context_line":"\t\treturn breakpoint_free(target, breakpoint);"},{"line_number":317,"context_line":"\t} else {"},{"line_number":318,"context_line":"\t\tif (!target-\u003esmp)"},{"line_number":319,"context_line":"\t\t\tLOG_TARGET_ERROR(target, \"no breakpoint found at address\" TARGET_ADDR_FMT, address);"},{"line_number":320,"context_line":"\t\treturn ERROR_BREAKPOINT_NOT_FOUND;"},{"line_number":321,"context_line":"\t}"},{"line_number":322,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":10,"id":"54e3f305_29fa833b","line":319,"in_reply_to":"37fd6a22_0fe85ef1","updated":"2023-08-14 11:39:33.000000000","message":"Done","commit_id":"d8001cef99716105a94896a916ca3cc280c11e9c"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"37235574ca3d8f6a6b24242a0bd702d997ed4675","unresolved":true,"context_lines":[{"line_number":316,"context_line":"\t\treturn breakpoint_free(target, breakpoint);"},{"line_number":317,"context_line":"\t} else {"},{"line_number":318,"context_line":"\t\tif (!target-\u003esmp)"},{"line_number":319,"context_line":"\t\t\tLOG_TARGET_ERROR(target, \"no breakpoint found at address\" TARGET_ADDR_FMT, address);"},{"line_number":320,"context_line":"\t\treturn ERROR_BREAKPOINT_NOT_FOUND;"},{"line_number":321,"context_line":"\t}"},{"line_number":322,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":10,"id":"37fd6a22_0fe85ef1","line":319,"in_reply_to":"e6af2e86_5b6dad6c","updated":"2023-08-10 10:16:49.000000000","message":"Addressed","commit_id":"d8001cef99716105a94896a916ca3cc280c11e9c"},{"author":{"_account_id":1000853,"name":"zapb","display_name":"Marc Schink","email":"dev@zapb.de","username":"zapb"},"change_message_id":"f3fbe8ae69702b7078ca4466639b6f68984b9c0d","unresolved":true,"context_lines":[{"line_number":538,"context_line":"\t\treturn ERROR_OK;"},{"line_number":539,"context_line":"\tretval \u003d target_remove_watchpoint(target, watchpoint);"},{"line_number":540,"context_line":"\tif (retval !\u003d ERROR_OK) {"},{"line_number":541,"context_line":"\t\tLOG_TARGET_ERROR(target, \"fail during running target-specific part of removing watchpoint %d\","},{"line_number":542,"context_line":"\t\t\t\t\t\twatchpoint-\u003enumber);"},{"line_number":543,"context_line":"\t\treturn retval;"},{"line_number":544,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":10,"id":"7f524276_633f4298","line":541,"updated":"2023-08-08 21:32:03.000000000","message":"\"failed to ...\" see below","commit_id":"d8001cef99716105a94896a916ca3cc280c11e9c"},{"author":{"_account_id":1000853,"name":"zapb","display_name":"Marc Schink","email":"dev@zapb.de","username":"zapb"},"change_message_id":"aed80c85e98705b6cea085547e214e0a4c39803d","unresolved":false,"context_lines":[{"line_number":538,"context_line":"\t\treturn ERROR_OK;"},{"line_number":539,"context_line":"\tretval \u003d target_remove_watchpoint(target, watchpoint);"},{"line_number":540,"context_line":"\tif (retval !\u003d ERROR_OK) {"},{"line_number":541,"context_line":"\t\tLOG_TARGET_ERROR(target, \"fail during running target-specific part of removing watchpoint %d\","},{"line_number":542,"context_line":"\t\t\t\t\t\twatchpoint-\u003enumber);"},{"line_number":543,"context_line":"\t\treturn retval;"},{"line_number":544,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":10,"id":"45d9d6ba_91067edd","line":541,"in_reply_to":"00a69461_2aeb396a","updated":"2023-08-14 11:39:33.000000000","message":"Done","commit_id":"d8001cef99716105a94896a916ca3cc280c11e9c"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"37235574ca3d8f6a6b24242a0bd702d997ed4675","unresolved":true,"context_lines":[{"line_number":538,"context_line":"\t\treturn ERROR_OK;"},{"line_number":539,"context_line":"\tretval \u003d target_remove_watchpoint(target, watchpoint);"},{"line_number":540,"context_line":"\tif (retval !\u003d ERROR_OK) {"},{"line_number":541,"context_line":"\t\tLOG_TARGET_ERROR(target, \"fail during running target-specific part of removing watchpoint %d\","},{"line_number":542,"context_line":"\t\t\t\t\t\twatchpoint-\u003enumber);"},{"line_number":543,"context_line":"\t\treturn retval;"},{"line_number":544,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":10,"id":"00a69461_2aeb396a","line":541,"in_reply_to":"7f524276_633f4298","updated":"2023-08-10 10:16:49.000000000","message":"Made it more consistent with line 219 (https://review.openocd.org/c/openocd/+/7738/comment/6d26edbf_5c3e9d59/)","commit_id":"d8001cef99716105a94896a916ca3cc280c11e9c"},{"author":{"_account_id":1000853,"name":"zapb","display_name":"Marc Schink","email":"dev@zapb.de","username":"zapb"},"change_message_id":"f3fbe8ae69702b7078ca4466639b6f68984b9c0d","unresolved":true,"context_lines":[{"line_number":583,"context_line":"\t\t\tif (status \u003d\u003d ERROR_OK) {"},{"line_number":584,"context_line":"\t\t\t\tnum_removed_watchpoints++;"},{"line_number":585,"context_line":"\t\t\t} else {"},{"line_number":586,"context_line":"\t\t\t\tLOG_TARGET_ERROR(curr, \"fail during removing watchpoint at address\" TARGET_ADDR_FMT, address);"},{"line_number":587,"context_line":"\t\t\t\tretval \u003d status;"},{"line_number":588,"context_line":"\t\t\t}"},{"line_number":589,"context_line":"\t\t}"}],"source_content_type":"text/x-csrc","patch_set":10,"id":"5e66246a_fc4cf995","line":586,"updated":"2023-08-08 21:32:03.000000000","message":"\"failed to remove watchpoint ...\", more consistent with current error messages","commit_id":"d8001cef99716105a94896a916ca3cc280c11e9c"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"37235574ca3d8f6a6b24242a0bd702d997ed4675","unresolved":true,"context_lines":[{"line_number":583,"context_line":"\t\t\tif (status \u003d\u003d ERROR_OK) {"},{"line_number":584,"context_line":"\t\t\t\tnum_removed_watchpoints++;"},{"line_number":585,"context_line":"\t\t\t} else {"},{"line_number":586,"context_line":"\t\t\t\tLOG_TARGET_ERROR(curr, \"fail during removing watchpoint at address\" TARGET_ADDR_FMT, address);"},{"line_number":587,"context_line":"\t\t\t\tretval \u003d status;"},{"line_number":588,"context_line":"\t\t\t}"},{"line_number":589,"context_line":"\t\t}"}],"source_content_type":"text/x-csrc","patch_set":10,"id":"848fd37a_8e267cee","line":586,"in_reply_to":"5e66246a_fc4cf995","updated":"2023-08-10 10:16:49.000000000","message":"Addressed","commit_id":"d8001cef99716105a94896a916ca3cc280c11e9c"},{"author":{"_account_id":1000853,"name":"zapb","display_name":"Marc Schink","email":"dev@zapb.de","username":"zapb"},"change_message_id":"aed80c85e98705b6cea085547e214e0a4c39803d","unresolved":false,"context_lines":[{"line_number":583,"context_line":"\t\t\tif (status \u003d\u003d ERROR_OK) {"},{"line_number":584,"context_line":"\t\t\t\tnum_removed_watchpoints++;"},{"line_number":585,"context_line":"\t\t\t} else {"},{"line_number":586,"context_line":"\t\t\t\tLOG_TARGET_ERROR(curr, \"fail during removing watchpoint at address\" TARGET_ADDR_FMT, address);"},{"line_number":587,"context_line":"\t\t\t\tretval \u003d status;"},{"line_number":588,"context_line":"\t\t\t}"},{"line_number":589,"context_line":"\t\t}"}],"source_content_type":"text/x-csrc","patch_set":10,"id":"42aa684b_85e29afd","line":586,"in_reply_to":"848fd37a_8e267cee","updated":"2023-08-14 11:39:33.000000000","message":"Done","commit_id":"d8001cef99716105a94896a916ca3cc280c11e9c"},{"author":{"_account_id":1000853,"name":"zapb","display_name":"Marc Schink","email":"dev@zapb.de","username":"zapb"},"change_message_id":"f3fbe8ae69702b7078ca4466639b6f68984b9c0d","unresolved":true,"context_lines":[{"line_number":588,"context_line":"\t\t\t}"},{"line_number":589,"context_line":"\t\t}"},{"line_number":590,"context_line":"\t\tif (num_removed_watchpoints \u003d\u003d 0)"},{"line_number":591,"context_line":"\t\t\tLOG_TARGET_ERROR(target, \"no watchpoint found at address \" TARGET_ADDR_FMT, address);"},{"line_number":592,"context_line":"\t} else {"},{"line_number":593,"context_line":"\t\tretval \u003d watchpoint_remove_internal(target, address);"},{"line_number":594,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":10,"id":"f5224132_b7e569a9","line":591,"updated":"2023-08-08 21:32:03.000000000","message":"Should be consistent with the error message in 567","commit_id":"d8001cef99716105a94896a916ca3cc280c11e9c"},{"author":{"_account_id":1000853,"name":"zapb","display_name":"Marc Schink","email":"dev@zapb.de","username":"zapb"},"change_message_id":"aed80c85e98705b6cea085547e214e0a4c39803d","unresolved":false,"context_lines":[{"line_number":588,"context_line":"\t\t\t}"},{"line_number":589,"context_line":"\t\t}"},{"line_number":590,"context_line":"\t\tif (num_removed_watchpoints \u003d\u003d 0)"},{"line_number":591,"context_line":"\t\t\tLOG_TARGET_ERROR(target, \"no watchpoint found at address \" TARGET_ADDR_FMT, address);"},{"line_number":592,"context_line":"\t} else {"},{"line_number":593,"context_line":"\t\tretval \u003d watchpoint_remove_internal(target, address);"},{"line_number":594,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":10,"id":"03568a3d_566ae845","line":591,"in_reply_to":"cc23b873_5a463871","updated":"2023-08-14 11:39:33.000000000","message":"Done","commit_id":"d8001cef99716105a94896a916ca3cc280c11e9c"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"37235574ca3d8f6a6b24242a0bd702d997ed4675","unresolved":true,"context_lines":[{"line_number":588,"context_line":"\t\t\t}"},{"line_number":589,"context_line":"\t\t}"},{"line_number":590,"context_line":"\t\tif (num_removed_watchpoints \u003d\u003d 0)"},{"line_number":591,"context_line":"\t\t\tLOG_TARGET_ERROR(target, \"no watchpoint found at address \" TARGET_ADDR_FMT, address);"},{"line_number":592,"context_line":"\t} else {"},{"line_number":593,"context_line":"\t\tretval \u003d watchpoint_remove_internal(target, address);"},{"line_number":594,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":10,"id":"cc23b873_5a463871","line":591,"in_reply_to":"f5224132_b7e569a9","updated":"2023-08-10 10:16:49.000000000","message":"Addressed","commit_id":"d8001cef99716105a94896a916ca3cc280c11e9c"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"81b28aad7513c78ba595d9243d80de2917354157","unresolved":true,"context_lines":[{"line_number":588,"context_line":"\t\t\t}"},{"line_number":589,"context_line":"\t\t}"},{"line_number":590,"context_line":"\t\tif (num_removed_watchpoints \u003d\u003d 0)"},{"line_number":591,"context_line":"\t\t\tLOG_TARGET_ERROR(target, \"no watchpoint at address \" TARGET_ADDR_FMT \" found\", address);"},{"line_number":592,"context_line":"\t} else {"},{"line_number":593,"context_line":"\t\tretval \u003d watchpoint_remove_internal(target, address);"},{"line_number":594,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":11,"id":"a6551e5c_e84b92cc","line":591,"updated":"2023-08-10 09:22:01.000000000","message":"Overall, this merge request looks good, and I have two last remarks related to the error messages.\n\n1) As for the \"no watchpoint\" error message, I think the logic should be changed a little. We should **not** print it if the watchpoint has been found but could not have been removed. In other words, we should keep count of the found watchpoints, not removed watchpoints.\n\n2) Also it would be good to print all the warnings in the same function, regardless if it is SMP or non-SMP target.\n\nThe same applies to breakpoint_remove() function. It is the exact same case, so I won\u0027t make a separate comment on that.\n\nThis is how I would recommend to structure the code (quick proposal, might contain typos):\n\n\n\tif (target-\u003esmp) {\n\t\tunsigned int num_found_watchpoints \u003d 0;\n\t\tstruct target_list *head;\n\t\tforeach_smp_target(head, target-\u003esmp_targets) {\n\t\t\tstruct target *curr \u003d head-\u003etarget;\n\t\t\tint status \u003d watchpoint_remove_internal(curr, address);\n\t\t\tif (status !\u003d ERROR_WATCHPOINT_NOT_FOUND) {\n\t\t\t\tnum_found_watchpoints++;\n\t\t\t}\n\t\t\tif (status !\u003d ERROR_WATCHPOINT_NOT_FOUND \u0026\u0026 status !\u003d ERROR_OK) {\n\t\t\t\tLOG_TARGET_ERROR(curr, \"failed to remove watchpoint at address\" TARGET_ADDR_FMT, address);\n\t\t\t\tretval \u003d status;\n\t\t\t}\n\t\t}\n\t\tif (num_found_watchpoints \u003d\u003d 0)\n\t\t\tLOG_TARGET_ERROR(target, \"no watchpoint at address \" TARGET_ADDR_FMT \" found\", address);\n\t} else {\n\t\tretval \u003d watchpoint_remove_internal(target, address);\n                if (status !\u003d ERROR_OK) {\n                    if (status \u003d\u003d ERROR_WATCHPOINT_NOT_FOUND) {\n\t\t\tLOG_TARGET_ERROR(target, \"no watchpoint at address \" TARGET_ADDR_FMT \" found\", address);                    \n                    } else {\n\t\t\tLOG_TARGET_ERROR(curr, \"failed to remove watchpoint at address\" TARGET_ADDR_FMT, address);                    \n                    }\n                }\n\t}\n\t\nThe error print from watchpoint_remove_internal() can then be removed.","commit_id":"0008183770434c2b49701d989e6e2a0aafa1555f"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"201f1ff40b49306f7dab1282d2b4bd7f96b0dd28","unresolved":true,"context_lines":[{"line_number":588,"context_line":"\t\t\t}"},{"line_number":589,"context_line":"\t\t}"},{"line_number":590,"context_line":"\t\tif (num_removed_watchpoints \u003d\u003d 0)"},{"line_number":591,"context_line":"\t\t\tLOG_TARGET_ERROR(target, \"no watchpoint at address \" TARGET_ADDR_FMT \" found\", address);"},{"line_number":592,"context_line":"\t} else {"},{"line_number":593,"context_line":"\t\tretval \u003d watchpoint_remove_internal(target, address);"},{"line_number":594,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":11,"id":"ca23e62c_89ec4b5d","line":591,"in_reply_to":"a6551e5c_e84b92cc","updated":"2023-08-10 15:13:57.000000000","message":"Agree with you. I use your example, but change it a liitle.\nNow we print error messages only in case of removing error or in case of no watchpoints (breakpoints) found at this address. As a consequence, duplicate of error message from watchpoint_remove_interna (breakpoint_remove_internal)","commit_id":"0008183770434c2b49701d989e6e2a0aafa1555f"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"74b17734b210349415d5d89fc269ca6c59120985","unresolved":false,"context_lines":[{"line_number":588,"context_line":"\t\t\t}"},{"line_number":589,"context_line":"\t\t}"},{"line_number":590,"context_line":"\t\tif (num_removed_watchpoints \u003d\u003d 0)"},{"line_number":591,"context_line":"\t\t\tLOG_TARGET_ERROR(target, \"no watchpoint at address \" TARGET_ADDR_FMT \" found\", address);"},{"line_number":592,"context_line":"\t} else {"},{"line_number":593,"context_line":"\t\tretval \u003d watchpoint_remove_internal(target, address);"},{"line_number":594,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":11,"id":"4cc34a02_fc770dcc","line":591,"in_reply_to":"ca23e62c_89ec4b5d","updated":"2023-08-11 05:46:37.000000000","message":"Done","commit_id":"0008183770434c2b49701d989e6e2a0aafa1555f"}],"src/target/breakpoints.h":[{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"ad5db0f186925f396421d00f31a94c04361dbbda","unresolved":true,"context_lines":[{"line_number":48,"context_line":"\tint unique_id;"},{"line_number":49,"context_line":"};"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"void breakpoint_clear_target(struct target *target);"},{"line_number":52,"context_line":"int breakpoint_add(struct target *target,"},{"line_number":53,"context_line":"\t\ttarget_addr_t address, uint32_t length, enum breakpoint_type type);"},{"line_number":54,"context_line":"int context_breakpoint_add(struct target *target,"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"0b546cfd_b340d2b4","line":51,"updated":"2023-06-16 07:07:17.000000000","message":"It would be good to add the return value also to breakpoint_clear_target(), which is essentially doing very similar thing to breakpoint_remove_all().","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"040db8003947e75f28b5a7b2f49967b38e78d8bb","unresolved":true,"context_lines":[{"line_number":48,"context_line":"\tint unique_id;"},{"line_number":49,"context_line":"};"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"void breakpoint_clear_target(struct target *target);"},{"line_number":52,"context_line":"int breakpoint_add(struct target *target,"},{"line_number":53,"context_line":"\t\ttarget_addr_t address, uint32_t length, enum breakpoint_type type);"},{"line_number":54,"context_line":"int context_breakpoint_add(struct target *target,"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"da676664_c8c88fda","line":51,"in_reply_to":"0b546cfd_b340d2b4","updated":"2023-06-16 07:59:36.000000000","message":"Addressed","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"95182593642e5e82c9248ff8b5749f1120a6919a","unresolved":false,"context_lines":[{"line_number":48,"context_line":"\tint unique_id;"},{"line_number":49,"context_line":"};"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"void breakpoint_clear_target(struct target *target);"},{"line_number":52,"context_line":"int breakpoint_add(struct target *target,"},{"line_number":53,"context_line":"\t\ttarget_addr_t address, uint32_t length, enum breakpoint_type type);"},{"line_number":54,"context_line":"int context_breakpoint_add(struct target *target,"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"6eeeb30a_2b1d2092","line":51,"in_reply_to":"da676664_c8c88fda","updated":"2023-06-19 10:36:42.000000000","message":"Done","commit_id":"45843c49f83cbe70c500d2a9d52743f72f943bc8"}],"src/target/target.c":[{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"b83b01077305fafd77842c40fc2c85715daf6d54","unresolved":true,"context_lines":[{"line_number":4051,"context_line":"\t}"},{"line_number":4052,"context_line":""},{"line_number":4053,"context_line":"\tif (retval !\u003d ERROR_OK)"},{"line_number":4054,"context_line":"\t\tLOG_ERROR(\"Failure removing breakpoints\");"},{"line_number":4055,"context_line":""},{"line_number":4056,"context_line":"\treturn retval;"},{"line_number":4057,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":7,"id":"68633003_8b3cf3ad","line":4054,"updated":"2023-08-07 06:26:01.000000000","message":"Unless others disagree, for better clarity for the user, I\u0027d recommend to have here two different error messages - depending on if we are removing a single breakpoint or all of them:\n\n\"Error encountered during removal of all breakpoints. Some breakpoints may have remained set.\"\n\n\"Error during removal of breakpoint at address \" TARGET_ADDR_FMT","commit_id":"55ee148bc07719913881cd5650263840b57fae7b"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"eca59359710a11731d1eb1f827a2d67ceb2cf2d6","unresolved":true,"context_lines":[{"line_number":4051,"context_line":"\t}"},{"line_number":4052,"context_line":""},{"line_number":4053,"context_line":"\tif (retval !\u003d ERROR_OK)"},{"line_number":4054,"context_line":"\t\tLOG_ERROR(\"Failure removing breakpoints\");"},{"line_number":4055,"context_line":""},{"line_number":4056,"context_line":"\treturn retval;"},{"line_number":4057,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":7,"id":"fe5f8da4_eee95957","line":4054,"in_reply_to":"68633003_8b3cf3ad","updated":"2023-08-07 07:58:47.000000000","message":"Addressed","commit_id":"55ee148bc07719913881cd5650263840b57fae7b"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"0780a466bcb744ff9ac07545d032268671adb7dd","unresolved":false,"context_lines":[{"line_number":4051,"context_line":"\t}"},{"line_number":4052,"context_line":""},{"line_number":4053,"context_line":"\tif (retval !\u003d ERROR_OK)"},{"line_number":4054,"context_line":"\t\tLOG_ERROR(\"Failure removing breakpoints\");"},{"line_number":4055,"context_line":""},{"line_number":4056,"context_line":"\treturn retval;"},{"line_number":4057,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":7,"id":"3ba5376e_c88c6001","line":4054,"in_reply_to":"fe5f8da4_eee95957","updated":"2023-08-07 10:30:41.000000000","message":"Done","commit_id":"55ee148bc07719913881cd5650263840b57fae7b"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"b83b01077305fafd77842c40fc2c85715daf6d54","unresolved":true,"context_lines":[{"line_number":4136,"context_line":"\tint retval \u003d watchpoint_remove(target, addr);"},{"line_number":4137,"context_line":""},{"line_number":4138,"context_line":"\tif (retval !\u003d ERROR_OK)"},{"line_number":4139,"context_line":"\t\tLOG_ERROR(\"Failure removing watchpoints\");"},{"line_number":4140,"context_line":""},{"line_number":4141,"context_line":"\treturn retval;"},{"line_number":4142,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":7,"id":"46d018b6_c7cae0aa","line":4139,"updated":"2023-08-07 06:26:01.000000000","message":"This could be rephrased as:\n\n\"Error during removal of watchpoint at address \" TARGET_ADDR_FMT","commit_id":"55ee148bc07719913881cd5650263840b57fae7b"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"eca59359710a11731d1eb1f827a2d67ceb2cf2d6","unresolved":true,"context_lines":[{"line_number":4136,"context_line":"\tint retval \u003d watchpoint_remove(target, addr);"},{"line_number":4137,"context_line":""},{"line_number":4138,"context_line":"\tif (retval !\u003d ERROR_OK)"},{"line_number":4139,"context_line":"\t\tLOG_ERROR(\"Failure removing watchpoints\");"},{"line_number":4140,"context_line":""},{"line_number":4141,"context_line":"\treturn retval;"},{"line_number":4142,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":7,"id":"fbd3986e_3f4bfe76","line":4139,"in_reply_to":"46d018b6_c7cae0aa","updated":"2023-08-07 07:58:47.000000000","message":"Addressed","commit_id":"55ee148bc07719913881cd5650263840b57fae7b"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"0780a466bcb744ff9ac07545d032268671adb7dd","unresolved":false,"context_lines":[{"line_number":4136,"context_line":"\tint retval \u003d watchpoint_remove(target, addr);"},{"line_number":4137,"context_line":""},{"line_number":4138,"context_line":"\tif (retval !\u003d ERROR_OK)"},{"line_number":4139,"context_line":"\t\tLOG_ERROR(\"Failure removing watchpoints\");"},{"line_number":4140,"context_line":""},{"line_number":4141,"context_line":"\treturn retval;"},{"line_number":4142,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":7,"id":"3cec2d59_c778e212","line":4139,"in_reply_to":"fbd3986e_3f4bfe76","updated":"2023-08-07 10:30:41.000000000","message":"Done","commit_id":"55ee148bc07719913881cd5650263840b57fae7b"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"b83b01077305fafd77842c40fc2c85715daf6d54","unresolved":true,"context_lines":[{"line_number":4141,"context_line":"\treturn retval;"},{"line_number":4142,"context_line":"}"},{"line_number":4143,"context_line":""},{"line_number":4144,"context_line":""},{"line_number":4145,"context_line":"/**"},{"line_number":4146,"context_line":" * Translate a virtual address to a physical address."},{"line_number":4147,"context_line":" *"}],"source_content_type":"text/x-csrc","patch_set":7,"id":"eb24f3fc_6ff87aea","line":4144,"updated":"2023-08-07 06:26:01.000000000","message":"This seems to be an extra blank line.","commit_id":"55ee148bc07719913881cd5650263840b57fae7b"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"0780a466bcb744ff9ac07545d032268671adb7dd","unresolved":false,"context_lines":[{"line_number":4141,"context_line":"\treturn retval;"},{"line_number":4142,"context_line":"}"},{"line_number":4143,"context_line":""},{"line_number":4144,"context_line":""},{"line_number":4145,"context_line":"/**"},{"line_number":4146,"context_line":" * Translate a virtual address to a physical address."},{"line_number":4147,"context_line":" *"}],"source_content_type":"text/x-csrc","patch_set":7,"id":"5b28c641_ca431e47","line":4144,"in_reply_to":"48ef4d6c_def2770c","updated":"2023-08-07 10:30:41.000000000","message":"Done","commit_id":"55ee148bc07719913881cd5650263840b57fae7b"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"eca59359710a11731d1eb1f827a2d67ceb2cf2d6","unresolved":true,"context_lines":[{"line_number":4141,"context_line":"\treturn retval;"},{"line_number":4142,"context_line":"}"},{"line_number":4143,"context_line":""},{"line_number":4144,"context_line":""},{"line_number":4145,"context_line":"/**"},{"line_number":4146,"context_line":" * Translate a virtual address to a physical address."},{"line_number":4147,"context_line":" *"}],"source_content_type":"text/x-csrc","patch_set":7,"id":"48ef4d6c_def2770c","line":4144,"in_reply_to":"eb24f3fc_6ff87aea","updated":"2023-08-07 07:58:47.000000000","message":"Addressed","commit_id":"55ee148bc07719913881cd5650263840b57fae7b"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"ab8f60d891f27986915c1335c17c9497c1b53190","unresolved":true,"context_lines":[{"line_number":4045,"context_line":"\t\tretval \u003d breakpoint_remove_all(target);"},{"line_number":4046,"context_line":""},{"line_number":4047,"context_line":"\t\tif (retval !\u003d ERROR_OK)"},{"line_number":4048,"context_line":"\t\t\tLOG_ERROR(\"Error encountered during removal of all breakpoints. Some breakpoints may have remained set.\");"},{"line_number":4049,"context_line":"\t} else {"},{"line_number":4050,"context_line":"\t\ttarget_addr_t addr;"},{"line_number":4051,"context_line":"\t\tCOMMAND_PARSE_ADDRESS(CMD_ARGV[0], addr);"}],"source_content_type":"text/x-csrc","patch_set":14,"id":"b9659984_14f98640","line":4048,"updated":"2023-08-15 14:24:54.000000000","message":"This is a COMMAND_HANDLER().\nError message should be propagated through command_print(), not through LOG_XXX()\nSame below in this function and in next COMMAND_HANDLER(handle_rwp_command)","commit_id":"7094c6b80e6c87e66caf7f9f13af8edb7f1af7f0"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"065c06824701c6fed8eaf9a8814eea5084dd6f9a","unresolved":true,"context_lines":[{"line_number":4045,"context_line":"\t\tretval \u003d breakpoint_remove_all(target);"},{"line_number":4046,"context_line":""},{"line_number":4047,"context_line":"\t\tif (retval !\u003d ERROR_OK)"},{"line_number":4048,"context_line":"\t\t\tLOG_ERROR(\"Error encountered during removal of all breakpoints. Some breakpoints may have remained set.\");"},{"line_number":4049,"context_line":"\t} else {"},{"line_number":4050,"context_line":"\t\ttarget_addr_t addr;"},{"line_number":4051,"context_line":"\t\tCOMMAND_PARSE_ADDRESS(CMD_ARGV[0], addr);"}],"source_content_type":"text/x-csrc","patch_set":14,"id":"bb32ea00_31384a31","line":4048,"in_reply_to":"b9659984_14f98640","updated":"2023-08-17 09:53:08.000000000","message":"Addressed","commit_id":"7094c6b80e6c87e66caf7f9f13af8edb7f1af7f0"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"072d5d7f68f60c7e6daa064d9a647ff795c4bad1","unresolved":false,"context_lines":[{"line_number":4045,"context_line":"\t\tretval \u003d breakpoint_remove_all(target);"},{"line_number":4046,"context_line":""},{"line_number":4047,"context_line":"\t\tif (retval !\u003d ERROR_OK)"},{"line_number":4048,"context_line":"\t\t\tLOG_ERROR(\"Error encountered during removal of all breakpoints. Some breakpoints may have remained set.\");"},{"line_number":4049,"context_line":"\t} else {"},{"line_number":4050,"context_line":"\t\ttarget_addr_t addr;"},{"line_number":4051,"context_line":"\t\tCOMMAND_PARSE_ADDRESS(CMD_ARGV[0], addr);"}],"source_content_type":"text/x-csrc","patch_set":14,"id":"4775a8a6_0cc68d21","line":4048,"in_reply_to":"bb32ea00_31384a31","updated":"2023-08-26 13:39:45.000000000","message":"Done","commit_id":"7094c6b80e6c87e66caf7f9f13af8edb7f1af7f0"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"ab8f60d891f27986915c1335c17c9497c1b53190","unresolved":true,"context_lines":[{"line_number":4057,"context_line":"\t}"},{"line_number":4058,"context_line":""},{"line_number":4059,"context_line":"\tif (retval !\u003d ERROR_OK)"},{"line_number":4060,"context_line":"\t\tLOG_ERROR(\"Failure removing breakpoints\");"},{"line_number":4061,"context_line":""},{"line_number":4062,"context_line":"\treturn retval;"},{"line_number":4063,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":14,"id":"0035a141_600166e6","line":4060,"updated":"2023-08-15 14:24:54.000000000","message":"each branch of the if/then/else above already provide an error message.\nDo we need a further one?","commit_id":"7094c6b80e6c87e66caf7f9f13af8edb7f1af7f0"},{"author":{"_account_id":1002152,"name":"Kirill Radkin","email":"kirill.radkin@syntacore.com","username":"kr-sc"},"change_message_id":"065c06824701c6fed8eaf9a8814eea5084dd6f9a","unresolved":true,"context_lines":[{"line_number":4057,"context_line":"\t}"},{"line_number":4058,"context_line":""},{"line_number":4059,"context_line":"\tif (retval !\u003d ERROR_OK)"},{"line_number":4060,"context_line":"\t\tLOG_ERROR(\"Failure removing breakpoints\");"},{"line_number":4061,"context_line":""},{"line_number":4062,"context_line":"\treturn retval;"},{"line_number":4063,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":14,"id":"94093541_2fc6d236","line":4060,"in_reply_to":"0035a141_600166e6","updated":"2023-08-17 09:53:08.000000000","message":"Removed. Addressed","commit_id":"7094c6b80e6c87e66caf7f9f13af8edb7f1af7f0"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"072d5d7f68f60c7e6daa064d9a647ff795c4bad1","unresolved":false,"context_lines":[{"line_number":4057,"context_line":"\t}"},{"line_number":4058,"context_line":""},{"line_number":4059,"context_line":"\tif (retval !\u003d ERROR_OK)"},{"line_number":4060,"context_line":"\t\tLOG_ERROR(\"Failure removing breakpoints\");"},{"line_number":4061,"context_line":""},{"line_number":4062,"context_line":"\treturn retval;"},{"line_number":4063,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":14,"id":"868e04d7_fc7650d0","line":4060,"in_reply_to":"94093541_2fc6d236","updated":"2023-08-26 13:39:45.000000000","message":"Done","commit_id":"7094c6b80e6c87e66caf7f9f13af8edb7f1af7f0"}]}