)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"dcb80c8a78de1b3495d69988272f3e3f7cea964e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"84471517_0a166347","updated":"2022-11-29 11:29:58.000000000","message":"Daniel, one hint about simplifying the code.\nBut please check if it instead makes it worst.","commit_id":"f8d02070ce30c23a88882a5b3d96ec22dab42a0b"},{"author":{"_account_id":1001810,"name":"Daniel Anselmi","email":"danselmi@gmx.ch","username":"danselmi"},"change_message_id":"b874f589df956af5b07c03c8766bcbfc8e6fb925","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"3217bf38_29f97dcb","updated":"2022-12-19 12:36:46.000000000","message":"\u003e Patch Set 8: Verified-1\n\u003e \n\u003e Build Failed \n\u003e \n\u003e https://build.openocd.org/job/openocd-gerrit-build/16387/ : FAILURE\n\u003e \n\u003e https://build.openocd.org/job/openocd-gerrit/17183/ : SUCCESS\n\n","commit_id":"bf0f33e0c4cf59d4bdbf7da8485380085838aa54"},{"author":{"_account_id":1001810,"name":"Daniel Anselmi","email":"danselmi@gmx.ch","username":"danselmi"},"change_message_id":"2cefbb029fd745c3194e92ca4a5d31ab4c78da61","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":15,"id":"6888c586_a63aef36","updated":"2023-04-30 17:43:12.000000000","message":"Thanks for reviewing.\nI personally don\u0027t like the solution for the xilinx devices. \nMaybe the driver should have configurable command codes.\nThis way it will be possible to configure them in the tcl scripts.\nIt would also allow to change command codes for loading for example the virtex 6 devices which is not possible with the hard-coded commands. What do you think?","commit_id":"0dca2409ca5f08daf3b79ad646864920e459125b"},{"author":{"_account_id":1001810,"name":"Daniel Anselmi","email":"danselmi@gmx.ch","username":"danselmi"},"change_message_id":"6b5d9bb355b60209f827bea6a377b997bed9dd85","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"5aae7c89_b50556c2","in_reply_to":"6888c586_a63aef36","updated":"2023-05-26 06:35:47.000000000","message":"Done","commit_id":"0dca2409ca5f08daf3b79ad646864920e459125b"},{"author":{"_account_id":1001810,"name":"Daniel Anselmi","email":"danselmi@gmx.ch","username":"danselmi"},"change_message_id":"b586a9a996988faf5dcbc8c796f45c4fba4a65e2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":20,"id":"e4e047be_c52a5c6d","updated":"2023-07-03 09:30:41.000000000","message":"I think this is also ready now.\nSince not all devices have the same number of user registers and intel counts from 0 to 1 (all others from 1 to 2 or 4). Its not worth to have a common implementation in pld.c\nAgain, thanks Antonio for your support!","commit_id":"2ba7dfcac072350e122c35bb585ae2453ed9bcd8"},{"author":{"_account_id":1001964,"name":"Erhan Kurubas","display_name":"Erhan Kurubas","email":"erhan.kurubas@espressif.com","username":"erhankur"},"change_message_id":"4661bcdce21f4329a4921a1cf51476c53e208731","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":21,"id":"88dc1543_7f9515ff","updated":"2023-07-25 11:20:21.000000000","message":"@daniel I see 2 new Sparse tool warnings. You can consider to fix them.\n\n../src/pld/lattice.c:322:5: warning: symbol \u0027lattice_get_ipdbg_hub\u0027 was not declared. Should it be static?\n../src/pld/virtex2.c:350:30: warning: constant 0xffffffffffffffff is so big it is unsigned long","commit_id":"a27907aed1cd26bcbaac834343f08146fc8fa1fe"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"6820b5e34a7608cf5ec5800883c6c41b6a3128dd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"5fb0e794_f6b448ca","updated":"2023-07-08 21:30:56.000000000","message":"Daniel,\nwould you mind having a look at the issue found by clang\nhttps://build.openocd.org/job/openocd-clang/1165/clang/new\nI have added the details below\nI can patch it, but I would still need you to test it.","commit_id":"a27907aed1cd26bcbaac834343f08146fc8fa1fe"},{"author":{"_account_id":1001810,"name":"Daniel Anselmi","email":"danselmi@gmx.ch","username":"danselmi"},"change_message_id":"58e3400efd65d1bd715e3e06eabcfbf1d656199d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"713355fd_2b51db5a","updated":"2023-07-08 23:39:29.000000000","message":"Thanks for reviewing all my stuff!","commit_id":"a27907aed1cd26bcbaac834343f08146fc8fa1fe"},{"author":{"_account_id":1001810,"name":"Daniel Anselmi","email":"danselmi@gmx.ch","username":"danselmi"},"change_message_id":"beae5e579a43587a2127206d78ebb500be693c72","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"c14c7a60_4982b672","in_reply_to":"88dc1543_7f9515ff","updated":"2023-07-25 21:38:34.000000000","message":"Thanks for reporting to me!\npatch is ready to review: https://review.openocd.org/c/openocd/+/7828","commit_id":"a27907aed1cd26bcbaac834343f08146fc8fa1fe"}],"src/pld/efinix.c":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"dcb80c8a78de1b3495d69988272f3e3f7cea964e","unresolved":true,"context_lines":[{"line_number":215,"context_line":"\tif (!pld_device_info || !pld_device_info-\u003etap)"},{"line_number":216,"context_line":"\t\treturn ERROR_FAIL;"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"\thub-\u003etap \u003d pld_device_info-\u003etap;"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"\tswitch (user_num) {"},{"line_number":221,"context_line":"\tcase 1:"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"c6bbd545_5a73967a","line":218,"updated":"2022-11-29 11:29:58.000000000","message":"we need a dedicated function per family because \u0027tap\u0027 is in the private data \u0027struct pld_device::driver_priv\u0027.\nWhat about moving \u0027tap\u0027 in \u0027struct pld_device\u0027 ? Can be easily done or it\u0027s too invasive?\nThen, with \u0027tap\u0027 in the general code, what about adding to \u0027struct pld_device\u0027\n unsigned int trion_user_ir_codes[] \u003d {USER1, USER2, USER3, USER4};\n struct pld_driver trion_pld \u003d {\n   ...\n   .user_ir_codes \u003d trion_user_ir_codes;\n   .user_ir_size \u003d ARRAY_SIZE(trion_user_ir_codes);\n };\n\nand make this function generic in pld.c and valid for every family?","commit_id":"f8d02070ce30c23a88882a5b3d96ec22dab42a0b"},{"author":{"_account_id":1001810,"name":"Daniel Anselmi","email":"danselmi@gmx.ch","username":"danselmi"},"change_message_id":"b21932a9514e46e47372898789e7f98db6961663","unresolved":false,"context_lines":[{"line_number":215,"context_line":"\tif (!pld_device_info || !pld_device_info-\u003etap)"},{"line_number":216,"context_line":"\t\treturn ERROR_FAIL;"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"\thub-\u003etap \u003d pld_device_info-\u003etap;"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"\tswitch (user_num) {"},{"line_number":221,"context_line":"\tcase 1:"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"e73fd847_278593d4","line":218,"in_reply_to":"c6bbd545_5a73967a","updated":"2022-11-30 23:26:39.000000000","message":"Thanks for your feedback.\n\nAs you can see in the last two patch sets, xilinx and efinix dont have the same number of user_ir_codes nor do they have the same value for all families. So your proposal was right for the first version. Sorry.\n\nAnother thought why I didn\u0027t liked your solution: The currently supported FPGAs/PLDs all have a JTAG interface, but that\u0027s not the case for all FPGAs/CPLDs.\nSo moving the \u0027tap\u0027 into pld_device looks wrong.\n\nAgain, thanks for your effort to review my patches.","commit_id":"f8d02070ce30c23a88882a5b3d96ec22dab42a0b"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"940ba9e73d52a9a0164de68e754724dafa548e9d","unresolved":false,"context_lines":[{"line_number":215,"context_line":"\tif (!pld_device_info || !pld_device_info-\u003etap)"},{"line_number":216,"context_line":"\t\treturn ERROR_FAIL;"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"\thub-\u003etap \u003d pld_device_info-\u003etap;"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"\tswitch (user_num) {"},{"line_number":221,"context_line":"\tcase 1:"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"0fd513c9_9a98973f","line":218,"in_reply_to":"e73fd847_278593d4","updated":"2023-04-10 17:11:35.000000000","message":"Ok for the part of user_id code. This has to be recovered through a function.\nBut tap can be moved in struct pld_device and left as NULL for FPGA/CPLD that are not on JTAG.\nMaybe we can first complete and merge all the patches \"pld: add support for ..\", then check the impact of moving tap in struct pld_device as an independent patch, and at last, if it makes sense, returning on this.\nThe major risk I see, is that I loose you patches in my inbox, so feel free to ping me loudly if it happens again","commit_id":"f8d02070ce30c23a88882a5b3d96ec22dab42a0b"}],"src/server/ipdbg.c":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"6820b5e34a7608cf5ec5800883c6c41b6a3128dd","unresolved":true,"context_lines":[{"line_number":237,"context_line":"\t*hub \u003d NULL;"},{"line_number":238,"context_line":"\tstruct ipdbg_hub *new_hub \u003d calloc(1, sizeof(struct ipdbg_hub));"},{"line_number":239,"context_line":"\tif (!new_hub) {"},{"line_number":240,"context_line":"\t\tfree(virtual_ir);"},{"line_number":241,"context_line":"\t\tLOG_ERROR(\"Out of memory\");"},{"line_number":242,"context_line":"\t\treturn ERROR_FAIL;"},{"line_number":243,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":21,"id":"956f1074_add57bef","line":240,"updated":"2023-07-08 21:30:56.000000000","message":"Suddenly, clang returns an issue with this free().\nIt is here since 2 years, from http://review.openocd.org/5938 and suddenly found, probably due to an update in clang!\nAnyway, here we have free(virtual_ir) and we return ERROR_FAIL.\nLet\u0027s move below to the caller ...","commit_id":"a27907aed1cd26bcbaac834343f08146fc8fa1fe"},{"author":{"_account_id":1001810,"name":"Daniel Anselmi","email":"danselmi@gmx.ch","username":"danselmi"},"change_message_id":"58e3400efd65d1bd715e3e06eabcfbf1d656199d","unresolved":false,"context_lines":[{"line_number":237,"context_line":"\t*hub \u003d NULL;"},{"line_number":238,"context_line":"\tstruct ipdbg_hub *new_hub \u003d calloc(1, sizeof(struct ipdbg_hub));"},{"line_number":239,"context_line":"\tif (!new_hub) {"},{"line_number":240,"context_line":"\t\tfree(virtual_ir);"},{"line_number":241,"context_line":"\t\tLOG_ERROR(\"Out of memory\");"},{"line_number":242,"context_line":"\t\treturn ERROR_FAIL;"},{"line_number":243,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":21,"id":"f9fe70e7_d7a27141","line":240,"in_reply_to":"956f1074_add57bef","updated":"2023-07-08 23:39:29.000000000","message":"Ack","commit_id":"a27907aed1cd26bcbaac834343f08146fc8fa1fe"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"6820b5e34a7608cf5ec5800883c6c41b6a3128dd","unresolved":true,"context_lines":[{"line_number":631,"context_line":"\t\t\treturn ERROR_FAIL;"},{"line_number":632,"context_line":"\t\t}"},{"line_number":633,"context_line":"\t} else {"},{"line_number":634,"context_line":"\t\tint retval \u003d ipdbg_create_hub(tap, user_instruction, data_register_length, virtual_ir, \u0026hub);"},{"line_number":635,"context_line":"\t\tif (retval !\u003d ERROR_OK) {"},{"line_number":636,"context_line":"\t\t\tfree(virtual_ir);"},{"line_number":637,"context_line":"\t\t\treturn retval;"}],"source_content_type":"text/x-csrc","patch_set":21,"id":"64dfaa25_c2fe32ff","line":634,"updated":"2023-07-08 21:30:56.000000000","message":"this is the caller, and in case of error there is another free(virtual_ir) ! Double free()!\nBANG!\nTo avoid these type of issues, when possible the free() should be in the same function of the alloc(). At least the code review gets easier.\nSo free(virtual_ir) should be at the end of COMMAND_HANDLER(handle_ipdbg_command)","commit_id":"a27907aed1cd26bcbaac834343f08146fc8fa1fe"},{"author":{"_account_id":1001810,"name":"Daniel Anselmi","email":"danselmi@gmx.ch","username":"danselmi"},"change_message_id":"c5a4aa1149758e26351eabfd7679d729ecde63d9","unresolved":false,"context_lines":[{"line_number":631,"context_line":"\t\t\treturn ERROR_FAIL;"},{"line_number":632,"context_line":"\t\t}"},{"line_number":633,"context_line":"\t} else {"},{"line_number":634,"context_line":"\t\tint retval \u003d ipdbg_create_hub(tap, user_instruction, data_register_length, virtual_ir, \u0026hub);"},{"line_number":635,"context_line":"\t\tif (retval !\u003d ERROR_OK) {"},{"line_number":636,"context_line":"\t\t\tfree(virtual_ir);"},{"line_number":637,"context_line":"\t\t\treturn retval;"}],"source_content_type":"text/x-csrc","patch_set":21,"id":"2e93672b_5a393acb","line":634,"in_reply_to":"183dc517_9e0c1b75","updated":"2023-07-10 20:24:13.000000000","message":"This specific error is fixed in 7770.\nI was able to trigger valgrind on a double free before the fix but not afterwards.\nThere are memory leaks in ipdbg.c i case of shutdown before stopping the ipdbg server. I believe this is ok, or is there an event to catch for this case?\nCan we discuss in 7770?","commit_id":"a27907aed1cd26bcbaac834343f08146fc8fa1fe"},{"author":{"_account_id":1001810,"name":"Daniel Anselmi","email":"danselmi@gmx.ch","username":"danselmi"},"change_message_id":"58e3400efd65d1bd715e3e06eabcfbf1d656199d","unresolved":true,"context_lines":[{"line_number":631,"context_line":"\t\t\treturn ERROR_FAIL;"},{"line_number":632,"context_line":"\t\t}"},{"line_number":633,"context_line":"\t} else {"},{"line_number":634,"context_line":"\t\tint retval \u003d ipdbg_create_hub(tap, user_instruction, data_register_length, virtual_ir, \u0026hub);"},{"line_number":635,"context_line":"\t\tif (retval !\u003d ERROR_OK) {"},{"line_number":636,"context_line":"\t\t\tfree(virtual_ir);"},{"line_number":637,"context_line":"\t\t\treturn retval;"}],"source_content_type":"text/x-csrc","patch_set":21,"id":"90c2b0a6_250e2ae4","line":634,"in_reply_to":"64dfaa25_c2fe32ff","updated":"2023-07-08 23:39:29.000000000","message":"I will prepare a patch. The idea was to use the memory when needed by the hub and delete it in the other cases (stopping or failed starting). Maybe it is simpler to allocate a second copy when creating a new hub?","commit_id":"a27907aed1cd26bcbaac834343f08146fc8fa1fe"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"0a5ca91b6fd3ea25790c85c2fc6a652299de7e29","unresolved":true,"context_lines":[{"line_number":631,"context_line":"\t\t\treturn ERROR_FAIL;"},{"line_number":632,"context_line":"\t\t}"},{"line_number":633,"context_line":"\t} else {"},{"line_number":634,"context_line":"\t\tint retval \u003d ipdbg_create_hub(tap, user_instruction, data_register_length, virtual_ir, \u0026hub);"},{"line_number":635,"context_line":"\t\tif (retval !\u003d ERROR_OK) {"},{"line_number":636,"context_line":"\t\t\tfree(virtual_ir);"},{"line_number":637,"context_line":"\t\t\treturn retval;"}],"source_content_type":"text/x-csrc","patch_set":21,"id":"183dc517_9e0c1b75","line":634,"in_reply_to":"90c2b0a6_250e2ae4","updated":"2023-07-09 09:19:33.000000000","message":"Humm, yes, the flow is more complex than I though at a first look.\nFeel free to make a proposal, then we will check it.\nCan you run OpenOCD under valgrind in Linux to see if it triggers any issue or memory buffer left allocated at exit?","commit_id":"a27907aed1cd26bcbaac834343f08146fc8fa1fe"}]}