)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"24cabb92fbf3ed96b5a8819b2e74f966717db9f7","unresolved":true,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This commit fixes two issues:"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"1. It allows to use target-independent fallback even if the target does"},{"line_number":16,"context_line":"   not provide target-specific routine (I assume this is a bug)."},{"line_number":17,"context_line":"2. If target-specific routine fails, we print a warning indicating that"},{"line_number":18,"context_line":"  fallback is used. Otherwise we had quite a confusing messages:"},{"line_number":19,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"7a0ac9c4_80195082","line":16,"range":{"start_line":15,"start_character":0,"end_line":16,"end_character":64},"updated":"2024-08-17 05:11:59.000000000","message":"Should be followed by a patch removing target specific checksum memory stubs (created to work around this shortcoming). Would you mind to submit one?","commit_id":"e642b4c703247e7a446610dca075d0bb76975245"},{"author":{"_account_id":1002161,"name":"Anatoly P","email":"kupokupokupopo@gmail.com","username":"ecco_the_dolphin"},"change_message_id":"3f1db5d7ee98e808e25d6b95d5a4737e6ad3ce39","unresolved":true,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This commit fixes two issues:"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"1. It allows to use target-independent fallback even if the target does"},{"line_number":16,"context_line":"   not provide target-specific routine (I assume this is a bug)."},{"line_number":17,"context_line":"2. If target-specific routine fails, we print a warning indicating that"},{"line_number":18,"context_line":"  fallback is used. Otherwise we had quite a confusing messages:"},{"line_number":19,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"15b92deb_404a620a","line":16,"range":{"start_line":15,"start_character":0,"end_line":16,"end_character":64},"in_reply_to":"7a0ac9c4_80195082","updated":"2024-08-17 07:04:06.000000000","message":"I\u0027ll try submit one, sure","commit_id":"e642b4c703247e7a446610dca075d0bb76975245"}],"src/target/target.c":[{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"24cabb92fbf3ed96b5a8819b2e74f966717db9f7","unresolved":true,"context_lines":[{"line_number":2063,"context_line":""},{"line_number":2064,"context_line":"\tretval \u003d target_alloc_working_area_try(target, size, area);"},{"line_number":2065,"context_line":"\tif (retval \u003d\u003d ERROR_TARGET_RESOURCE_NOT_AVAILABLE)"},{"line_number":2066,"context_line":"\t\tLOG_TARGET_WARNING(target,"},{"line_number":2067,"context_line":"\t\t\t\t\"not enough working area available(requested %\" PRIu32 \")\", size);"},{"line_number":2068,"context_line":"\treturn retval;"},{"line_number":2069,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":2,"id":"a24a7e8a_cbd17b9e","line":2066,"range":{"start_line":2066,"start_character":6,"end_line":2066,"end_character":12},"updated":"2024-08-17 05:11:59.000000000","message":"I\u0027m not against, actually a good change... but not mentioned in the commit msg, not related with the described topic. Applies to lot of similar changes: wouldn\u0027t be worth to submit them in a separate patch?","commit_id":"e642b4c703247e7a446610dca075d0bb76975245"},{"author":{"_account_id":1002161,"name":"Anatoly P","email":"kupokupokupopo@gmail.com","username":"ecco_the_dolphin"},"change_message_id":"3f1db5d7ee98e808e25d6b95d5a4737e6ad3ce39","unresolved":true,"context_lines":[{"line_number":2063,"context_line":""},{"line_number":2064,"context_line":"\tretval \u003d target_alloc_working_area_try(target, size, area);"},{"line_number":2065,"context_line":"\tif (retval \u003d\u003d ERROR_TARGET_RESOURCE_NOT_AVAILABLE)"},{"line_number":2066,"context_line":"\t\tLOG_TARGET_WARNING(target,"},{"line_number":2067,"context_line":"\t\t\t\t\"not enough working area available(requested %\" PRIu32 \")\", size);"},{"line_number":2068,"context_line":"\treturn retval;"},{"line_number":2069,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":2,"id":"88273c4d_43166546","line":2066,"range":{"start_line":2066,"start_character":6,"end_line":2066,"end_character":12},"in_reply_to":"a24a7e8a_cbd17b9e","updated":"2024-08-17 07:04:06.000000000","message":"Ah.. I was thinking about this myself. It\u0027s just happened that the main functionality in question required working area to be set (on my target), so I could not stop myself to shove all this into one commit. I\u0027ll split the changeset in two.","commit_id":"e642b4c703247e7a446610dca075d0bb76975245"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"24cabb92fbf3ed96b5a8819b2e74f966717db9f7","unresolved":true,"context_lines":[{"line_number":2467,"context_line":""},{"line_number":2468,"context_line":"int target_checksum_memory(struct target *target, target_addr_t address, uint32_t size, uint32_t *crc)"},{"line_number":2469,"context_line":"{"},{"line_number":2470,"context_line":"\tassert(target);"},{"line_number":2471,"context_line":"\tassert(crc);"},{"line_number":2472,"context_line":"\tif (!target_was_examined(target)) {"},{"line_number":2473,"context_line":"\t\tLOG_ERROR(\"Target not examined yet\");"},{"line_number":2474,"context_line":"\t\treturn ERROR_FAIL;"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"50f7a11d_72ac0d07","line":2471,"range":{"start_line":2470,"start_character":1,"end_line":2471,"end_character":13},"updated":"2024-08-17 05:11:59.000000000","message":"Please do not!\nImagine each OpenOCD function would assert on each pointer parameter.\nAll modern operating systems reliably detect null pointer dereference without an assert.","commit_id":"e642b4c703247e7a446610dca075d0bb76975245"},{"author":{"_account_id":1002161,"name":"Anatoly P","email":"kupokupokupopo@gmail.com","username":"ecco_the_dolphin"},"change_message_id":"3f1db5d7ee98e808e25d6b95d5a4737e6ad3ce39","unresolved":true,"context_lines":[{"line_number":2467,"context_line":""},{"line_number":2468,"context_line":"int target_checksum_memory(struct target *target, target_addr_t address, uint32_t size, uint32_t *crc)"},{"line_number":2469,"context_line":"{"},{"line_number":2470,"context_line":"\tassert(target);"},{"line_number":2471,"context_line":"\tassert(crc);"},{"line_number":2472,"context_line":"\tif (!target_was_examined(target)) {"},{"line_number":2473,"context_line":"\t\tLOG_ERROR(\"Target not examined yet\");"},{"line_number":2474,"context_line":"\t\treturn ERROR_FAIL;"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"04db10a4_7ec675e0","line":2471,"range":{"start_line":2470,"start_character":1,"end_line":2471,"end_character":13},"in_reply_to":"50f7a11d_72ac0d07","updated":"2024-08-17 07:04:06.000000000","message":"I personally don\u0027t mind having those in each and every function since these provide better diagnostics in case when things go awry (instead of silently crashing) and these are mostly free... \n\nBut I see your point - probably, we should not disturb the existing codebase too match. And I think that this is an opinionated topic borderline with codestyle issues :) . That is - I don\u0027t mind removing these assert statements.","commit_id":"e642b4c703247e7a446610dca075d0bb76975245"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"24cabb92fbf3ed96b5a8819b2e74f966717db9f7","unresolved":true,"context_lines":[{"line_number":2473,"context_line":"\t\tLOG_ERROR(\"Target not examined yet\");"},{"line_number":2474,"context_line":"\t\treturn ERROR_FAIL;"},{"line_number":2475,"context_line":"\t}"},{"line_number":2476,"context_line":"\tif (!target-\u003etype-\u003echecksum_memory) {"},{"line_number":2477,"context_line":"\t\tLOG_ERROR(\"Target %s doesn\u0027t support checksum_memory\", target_name(target));"},{"line_number":2478,"context_line":"\t\treturn ERROR_FAIL;"},{"line_number":2479,"context_line":"\t}"},{"line_number":2480,"context_line":""},{"line_number":2481,"context_line":"\tif (target-\u003etype-\u003echecksum_memory) {"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"6ce2ad3b_09650ed4","line":2478,"range":{"start_line":2476,"start_character":1,"end_line":2478,"end_character":20},"updated":"2024-08-17 05:11:59.000000000","message":"Your commit msg reads: \"it allows to use target-independent fallback even if the target does not provide target-specific routine\"\n\nI see that both patched and unpatched code return fail. How did you test it?","commit_id":"e642b4c703247e7a446610dca075d0bb76975245"},{"author":{"_account_id":1002161,"name":"Anatoly P","email":"kupokupokupopo@gmail.com","username":"ecco_the_dolphin"},"change_message_id":"3f1db5d7ee98e808e25d6b95d5a4737e6ad3ce39","unresolved":true,"context_lines":[{"line_number":2473,"context_line":"\t\tLOG_ERROR(\"Target not examined yet\");"},{"line_number":2474,"context_line":"\t\treturn ERROR_FAIL;"},{"line_number":2475,"context_line":"\t}"},{"line_number":2476,"context_line":"\tif (!target-\u003etype-\u003echecksum_memory) {"},{"line_number":2477,"context_line":"\t\tLOG_ERROR(\"Target %s doesn\u0027t support checksum_memory\", target_name(target));"},{"line_number":2478,"context_line":"\t\treturn ERROR_FAIL;"},{"line_number":2479,"context_line":"\t}"},{"line_number":2480,"context_line":""},{"line_number":2481,"context_line":"\tif (target-\u003etype-\u003echecksum_memory) {"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"46afad4c_759c5cb0","line":2478,"range":{"start_line":2476,"start_character":1,"end_line":2478,"end_character":20},"in_reply_to":"6ce2ad3b_09650ed4","updated":"2024-08-17 07:04:06.000000000","message":"Yes, this looks like a mistake on my part.\n\nI\u0027ve tested it on a target that **supports** target-specific checksum_memory function but it requires working area to be set. The logs in the commit message are from that target. (It was a spike simulator/riscv target).\n\nI\u0027ll patch-out OpenOCD sources and will double check the case when checksum_memory is not implemented.","commit_id":"e642b4c703247e7a446610dca075d0bb76975245"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"24cabb92fbf3ed96b5a8819b2e74f966717db9f7","unresolved":true,"context_lines":[{"line_number":2484,"context_line":"\t\t\treturn ERROR_OK;"},{"line_number":2485,"context_line":""},{"line_number":2486,"context_line":"\t\tLOG_TARGET_WARNING(target,"},{"line_number":2487,"context_line":"\t\t\t\t\"target-specific checksum_memory routine failed, attempting generic \""},{"line_number":2488,"context_line":"\t\t\t\t\"routine as a fallback\");"},{"line_number":2489,"context_line":"\t} else {"},{"line_number":2490,"context_line":"\t\tLOG_TARGET_WARNING(target,"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"68caa656_12bc3b93","line":2487,"updated":"2024-08-17 05:11:59.000000000","message":"The problem was in the original code, getting little bit worse in the patched code:\nthe verbosity could be annoying because target_checksum_memory() is called for each image section. So if a verified image has 10 sections you see all the messages 10 times","commit_id":"e642b4c703247e7a446610dca075d0bb76975245"},{"author":{"_account_id":1002161,"name":"Anatoly P","email":"kupokupokupopo@gmail.com","username":"ecco_the_dolphin"},"change_message_id":"3f1db5d7ee98e808e25d6b95d5a4737e6ad3ce39","unresolved":true,"context_lines":[{"line_number":2484,"context_line":"\t\t\treturn ERROR_OK;"},{"line_number":2485,"context_line":""},{"line_number":2486,"context_line":"\t\tLOG_TARGET_WARNING(target,"},{"line_number":2487,"context_line":"\t\t\t\t\"target-specific checksum_memory routine failed, attempting generic \""},{"line_number":2488,"context_line":"\t\t\t\t\"routine as a fallback\");"},{"line_number":2489,"context_line":"\t} else {"},{"line_number":2490,"context_line":"\t\tLOG_TARGET_WARNING(target,"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"687a82a9_ffc86532","line":2487,"in_reply_to":"68caa656_12bc3b93","updated":"2024-08-17 07:04:06.000000000","message":"I missed this part :(. I was testing on a file that had only one program segment to load. I\u0027ll try to come up with something better.","commit_id":"e642b4c703247e7a446610dca075d0bb76975245"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"24cabb92fbf3ed96b5a8819b2e74f966717db9f7","unresolved":true,"context_lines":[{"line_number":2503,"context_line":"\t\treturn retval;"},{"line_number":2504,"context_line":"\t}"},{"line_number":2505,"context_line":""},{"line_number":2506,"context_line":"\t/* convert to target endianness */"},{"line_number":2507,"context_line":"\t/* FIXME: we should handle cases when size is not multiple of sizeof(uint32_t) */"},{"line_number":2508,"context_line":"\tfor (uint32_t i \u003d 0; i \u003c (size / sizeof(uint32_t)); i++) {"},{"line_number":2509,"context_line":"\t\tuint32_t target_data;"},{"line_number":2510,"context_line":"\t\ttarget_data \u003d target_buffer_get_u32(target, \u0026buffer[i * sizeof(uint32_t)]);"},{"line_number":2511,"context_line":"\t\ttarget_buffer_set_u32(target, \u0026buffer[i * sizeof(uint32_t)], target_data);"},{"line_number":2512,"context_line":"\t}"},{"line_number":2513,"context_line":""},{"line_number":2514,"context_line":"\tretval \u003d image_calculate_checksum(buffer, size, crc);"},{"line_number":2515,"context_line":"\tfree(buffer);"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"4d6276cc_d42c429c","line":2512,"range":{"start_line":2506,"start_character":1,"end_line":2512,"end_character":2},"updated":"2024-08-17 05:11:59.000000000","message":"I know you added just the FIXME comment to this old crap code\n(since 2007, commit e27696f6b04459e935a0a5f65f7f668cb02970dd )\nDid not anybody noticed that converting to host endianness and then back to target is the same as doing nothing??!!! CRC is computed on byte buffer not on words, no endianness conversion actually needed!","commit_id":"e642b4c703247e7a446610dca075d0bb76975245"},{"author":{"_account_id":1002161,"name":"Anatoly P","email":"kupokupokupopo@gmail.com","username":"ecco_the_dolphin"},"change_message_id":"3f1db5d7ee98e808e25d6b95d5a4737e6ad3ce39","unresolved":true,"context_lines":[{"line_number":2503,"context_line":"\t\treturn retval;"},{"line_number":2504,"context_line":"\t}"},{"line_number":2505,"context_line":""},{"line_number":2506,"context_line":"\t/* convert to target endianness */"},{"line_number":2507,"context_line":"\t/* FIXME: we should handle cases when size is not multiple of sizeof(uint32_t) */"},{"line_number":2508,"context_line":"\tfor (uint32_t i \u003d 0; i \u003c (size / sizeof(uint32_t)); i++) {"},{"line_number":2509,"context_line":"\t\tuint32_t target_data;"},{"line_number":2510,"context_line":"\t\ttarget_data \u003d target_buffer_get_u32(target, \u0026buffer[i * sizeof(uint32_t)]);"},{"line_number":2511,"context_line":"\t\ttarget_buffer_set_u32(target, \u0026buffer[i * sizeof(uint32_t)], target_data);"},{"line_number":2512,"context_line":"\t}"},{"line_number":2513,"context_line":""},{"line_number":2514,"context_line":"\tretval \u003d image_calculate_checksum(buffer, size, crc);"},{"line_number":2515,"context_line":"\tfree(buffer);"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"40410550_df7001f2","line":2512,"range":{"start_line":2506,"start_character":1,"end_line":2512,"end_character":2},"in_reply_to":"4d6276cc_d42c429c","updated":"2024-08-17 07:04:06.000000000","message":":) oh. I\u0027ve just observed that there is some weird things with endianess and the size of input data. I could not even imagine that it was THAT bad.","commit_id":"e642b4c703247e7a446610dca075d0bb76975245"}]}