)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1001242,"name":"Tim Newsome","email":"tim@sifive.com","username":"timsifive"},"change_message_id":"2b726005ed2e44a0f4c3478832f19ad482448a2b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6efc5a58_87b22fa9","updated":"2022-06-27 17:03:36.000000000","message":"On 64-bit targets, length could easily be more than 64 bits. I think target_addr_t is a better type to use than uint32_t.","commit_id":"46b94ac7bc49b00fd33bcc46841c28f5242cbd40"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"a1885f5729837e4db3a002953396a2dfc565a665","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"f78d6e4e_279b2d8a","updated":"2022-06-27 21:50:35.000000000","message":"Tim, the field \u0027length\u0027 is the number of bytes of the opcode of the instruction that has to be either \u0027covered\u0027 by an HW breakpoint or \u0027replaced\u0027 by a SW breakpoint.\nI don\u0027t expect it to require a 64 bit container.\n\nMarc, the uint32_t should be used when it matches the size 32 bits of an object on the target (register, memory area, ...).\nHere there is no such match. Should a generic \u0027unsigned int\u0027 be more appropriate?\n","commit_id":"46b94ac7bc49b00fd33bcc46841c28f5242cbd40"},{"author":{"_account_id":1000853,"name":"zapb","display_name":"Marc Schink","email":"dev@zapb.de","username":"zapb"},"change_message_id":"1410dcaee6fde0bc703348352d8d52603d381ccc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"23c67d6a_7cdab287","in_reply_to":"2302845c_542beae3","updated":"2022-06-29 22:57:43.000000000","message":"\u003e Marc, the uint32_t should be used when it matches the size 32 bits of an object\n\u003e on the target (register, memory area, ...).\n\u003e Here there is no such match. Should a generic \u0027unsigned int\u0027 be more appropriate?\n\nYep, makes sense!","commit_id":"46b94ac7bc49b00fd33bcc46841c28f5242cbd40"},{"author":{"_account_id":1000853,"name":"zapb","display_name":"Marc Schink","email":"dev@zapb.de","username":"zapb"},"change_message_id":"1410dcaee6fde0bc703348352d8d52603d381ccc","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c926b0b1_053a2280","in_reply_to":"6efc5a58_87b22fa9","updated":"2022-06-29 22:57:43.000000000","message":"Currently, uint32_t is used everywhere in the code. So this patch just adapts the last variable.\n\nBut apart from that: \u0027length\u0027 encodes the breakpoint length in bytes if I am not mistaken. A uint32_t should be sufficient in my opinion. What do you think?","commit_id":"46b94ac7bc49b00fd33bcc46841c28f5242cbd40"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"c8c221c6f18b16768887606c92b018001fcc374c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"9dd2d0a3_4c893e6d","in_reply_to":"c926b0b1_053a2280","updated":"2024-08-02 13:21:27.000000000","message":"Ack","commit_id":"46b94ac7bc49b00fd33bcc46841c28f5242cbd40"},{"author":{"_account_id":1001242,"name":"Tim Newsome","email":"tim@sifive.com","username":"timsifive"},"change_message_id":"90407004095a7e55e7ac4339e357ba92d298ba12","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2302845c_542beae3","in_reply_to":"f78d6e4e_279b2d8a","updated":"2022-06-28 17:24:17.000000000","message":"Ah, I had it confused with hardware breakpoint. Thanks for explaining.","commit_id":"46b94ac7bc49b00fd33bcc46841c28f5242cbd40"},{"author":{"_account_id":1000853,"name":"zapb","display_name":"Marc Schink","email":"dev@zapb.de","username":"zapb"},"change_message_id":"657bd354b3556c60d76c78799669463265bf5afd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9318bdf3_3bf18a50","updated":"2024-07-30 21:25:01.000000000","message":"Let\u0027s merge this after I pushed a new patchset with fixed format strings from the other patch.","commit_id":"d7a7b251c2157287e276dce333ee1440b4085cf8"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"7af9adb4a189293e0619260ea05a00611f48aa6d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"2a5e03fa_56c05f3d","updated":"2022-06-29 09:49:36.000000000","message":"Thanks","commit_id":"d7a7b251c2157287e276dce333ee1440b4085cf8"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"c8c221c6f18b16768887606c92b018001fcc374c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"b504a072_3f4c8b99","updated":"2024-08-02 13:21:27.000000000","message":"patch approved and then lost for two years!\nwe are really running out of time...\nThanks for the v3","commit_id":"19a11e876f38854b14a7a915a06f426e5f3e4a02"}],"src/target/breakpoints.c":[{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"4489f9e6dc3762c62f082246f9d42eb5cd5736c9","unresolved":true,"context_lines":[{"line_number":500,"context_line":"}"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"static int watchpoint_add_internal(struct target *target, target_addr_t address,"},{"line_number":503,"context_line":"\t\tunsigned int length, enum watchpoint_rw rw, uint32_t value, uint32_t mask)"},{"line_number":504,"context_line":"{"},{"line_number":505,"context_line":"\tstruct watchpoint *watchpoint \u003d target-\u003ewatchpoints;"},{"line_number":506,"context_line":"\tstruct watchpoint **watchpoint_p \u003d \u0026target-\u003ewatchpoints;"}],"source_content_type":"text/x-csrc","patch_set":4,"id":"3c4d9185_83614e2a","line":503,"updated":"2024-09-16 14:55:29.000000000","message":"I believe this to be a missmanaged conflict with https://review.openocd.org/c/openocd/+/7840.\n\n`value` and `mask` should be `uint64_t`.\n\nCurrent version introduces a bug:\n`WATCHPOINT_IGNORE_DATA_VALUE_MASK` is defined as `(~(uint64_t)0)`.\nTruncation to `uint32_t` makes it so the comparisons with the constant don\u0027t work.\nE.g. the one in `src/target/armv8_dpm.c`, in `dpmv8_watchpoint_setup()` on line 1220.\nhttps://review.openocd.org/c/openocd/+/7056/4/src/target/armv8_dpm.c#1220","commit_id":"0bf3373e808a097fdf50fc04f987c26b35ddf09d"},{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"c67558b7480f8ea77934b308c332cabe4eb737f6","unresolved":true,"context_lines":[{"line_number":500,"context_line":"}"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"static int watchpoint_add_internal(struct target *target, target_addr_t address,"},{"line_number":503,"context_line":"\t\tunsigned int length, enum watchpoint_rw rw, uint32_t value, uint32_t mask)"},{"line_number":504,"context_line":"{"},{"line_number":505,"context_line":"\tstruct watchpoint *watchpoint \u003d target-\u003ewatchpoints;"},{"line_number":506,"context_line":"\tstruct watchpoint **watchpoint_p \u003d \u0026target-\u003ewatchpoints;"}],"source_content_type":"text/x-csrc","patch_set":4,"id":"03cdeff4_e162dcb1","line":503,"in_reply_to":"3c4d9185_83614e2a","updated":"2024-09-16 15:15:30.000000000","message":"Please, see the suggested fix in https://review.openocd.org/c/openocd/+/8500.","commit_id":"0bf3373e808a097fdf50fc04f987c26b35ddf09d"}],"src/target/breakpoints.h":[{"author":{"_account_id":1001242,"name":"Tim Newsome","email":"tim@sifive.com","username":"timsifive"},"change_message_id":"dd97a743fb23e37f85dd7ee0bfe86a2a305f41f1","unresolved":true,"context_lines":[{"line_number":49,"context_line":""},{"line_number":50,"context_line":"struct watchpoint {"},{"line_number":51,"context_line":"\ttarget_addr_t address;"},{"line_number":52,"context_line":"\tunsigned int length;"},{"line_number":53,"context_line":"\tuint32_t mask;"},{"line_number":54,"context_line":"\tuint32_t value;"},{"line_number":55,"context_line":"\tenum watchpoint_rw rw;"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"ae96f9b7_766b349d","line":52,"updated":"2022-06-29 19:45:39.000000000","message":"Now *this* can be a proper address range on 64-bit targets, right? Does this need to be target_addr_t?","commit_id":"d7a7b251c2157287e276dce333ee1440b4085cf8"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"0bd72ce4edeb3d1bd3533e065b435703c931f343","unresolved":true,"context_lines":[{"line_number":49,"context_line":""},{"line_number":50,"context_line":"struct watchpoint {"},{"line_number":51,"context_line":"\ttarget_addr_t address;"},{"line_number":52,"context_line":"\tunsigned int length;"},{"line_number":53,"context_line":"\tuint32_t mask;"},{"line_number":54,"context_line":"\tuint32_t value;"},{"line_number":55,"context_line":"\tenum watchpoint_rw rw;"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"b31c1983_26b280f0","line":52,"in_reply_to":"ae96f9b7_766b349d","updated":"2022-06-30 07:43:35.000000000","message":"Hummm, yes.\nWhile I\u0027m not aware of any \"wide range\" breakpoint, watchpoint can easily cover a wide area. Typically in HW it is expressed as log2 of the length, so max in [0..63], but here we have length in bytes...\nIn this patch or in a separate one?","commit_id":"d7a7b251c2157287e276dce333ee1440b4085cf8"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"c8c221c6f18b16768887606c92b018001fcc374c","unresolved":false,"context_lines":[{"line_number":49,"context_line":""},{"line_number":50,"context_line":"struct watchpoint {"},{"line_number":51,"context_line":"\ttarget_addr_t address;"},{"line_number":52,"context_line":"\tunsigned int length;"},{"line_number":53,"context_line":"\tuint32_t mask;"},{"line_number":54,"context_line":"\tuint32_t value;"},{"line_number":55,"context_line":"\tenum watchpoint_rw rw;"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"58ca7471_2593a979","line":52,"in_reply_to":"b31c1983_26b280f0","updated":"2024-08-02 13:21:27.000000000","message":"Ack","commit_id":"d7a7b251c2157287e276dce333ee1440b4085cf8"}]}