)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"13c2d8c9f3b5731bbc7fbbbafb74088c81935450","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"767f0702_593a5ba9","updated":"2023-12-10 16:47:17.000000000","message":"Functionality on macOS confirmed by reporter:\nhttps://sourceforge.net/p/openocd/mailman/openocd-devel/thread/FE5CD81D-D629-40A5-875E-ED430BA34A9E%40onethinx.com/#msg58711332","commit_id":"fbe13754e27bb3f3619cb2891f99be8c458eb877"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"ab7cbee540664a26c93b1d3e5d5a7bb3893e738c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"a3a8d0dd_4a80aec6","updated":"2023-12-10 17:27:23.000000000","message":"What\u0027s wrong with patchset 1 that requires you to add all those ugly #if ?\n\nWe have src/jtag/drivers/libusb_helper.[ch]\nI agree they are ugly too because of the jtag_libusb_ prefix, but those can be easily renamed.\n\nWhat about moving there:\n\n#if !defined(LIBUSB_API_VERSION) || (LIBUSB_API_VERSION \u003c 0x01000105)\n\t#define libusb_dev_mem_alloc(dev, sz) malloc(sz)\n\t#define libusb_dev_mem_free(dev, buffer, sz) free(buffer)\n#endif\n\nunsigned char *jtag_libusb_dev_mem_alloc(...)\n{\n        unsigned char *x \u003d libusb_dev_mem_alloc(...);\n        if (!x)\n                 x \u003d malloc(...);\n        return x;\n}\n\nvoid jtag_libusb_dev_mem_free(...)\n{\n        int x \u003d libusb_dev_mem_free(...);\n        if (x \u003d\u003d LIBUSB_ERROR_NOT_SUPPORTED)\n                 free(...);\n}\n\nI think that other drivers can benefit from such centralization, once someone try to optimize them.","commit_id":"fbe13754e27bb3f3619cb2891f99be8c458eb877"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"55d6425553ae7cd569eb18bafb3985e0948a68e2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"c2968825_49d1af3e","in_reply_to":"a3a8d0dd_4a80aec6","updated":"2023-12-11 11:33:01.000000000","message":"\u003e What\u0027s wrong with patchset 1 that requires you to add all those ugly #if ?\n\nIt pre-compiles with an old libusb to\n```\n  ptr\u003d malloc(sz);\n  if (!ptr)\n    ptr\u003d malloc(sz);\n```\nAlthough functional, IMO little bit silly.\nYour proposal does the same.\n\n\u003e unsigned char *jtag_libusb_dev_mem_alloc(...)\n\u003e {\n\u003e         unsigned char *x \u003d libusb_dev_mem_alloc(...);\n\u003e         if (!x)\n\u003e                  x \u003d malloc(...);\n\u003e         return x;\n\u003e }\n\nIn corner case it could allocate any mixture of dev_mem blocks and standard heap blocks.\n\n\u003e void jtag_libusb_dev_mem_free(...)\n\u003e {\n\u003e         int x \u003d libusb_dev_mem_free(...);\n\u003e         if (x \u003d\u003d LIBUSB_ERROR_NOT_SUPPORTED)\n\u003e                  free(...);\n\u003e }\n\nAre you sure that one can try to free malloced block with libusb_dev_mem_free()? libusb doc speaks clearly:\nFree device memory **allocated with libusb_dev_mem_alloc()**.\n\nWill not libusb_dev_mem_free() corrupt the heap or emit a segfault?\n\n\u003e I think that other drivers can benefit from such centralization, once someone try to optimize them.\n\nI think this could be done later. I wanted to quickly fix the serious malfunction on macOS. I have an idea how to get rid of most #if #endif but don\u0027t have time to re-write and test the code. Leaving for holidays today, will not be able to continue until new year.","commit_id":"fbe13754e27bb3f3619cb2891f99be8c458eb877"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"958a6b3487e83faf537615a5abb09603a7b184a8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"6da032dd_6ea68754","updated":"2023-12-29 14:04:21.000000000","message":"Edited the commit message to re-run jenkins build (The problem was caused by #8059 which this one depends on)","commit_id":"f1e185478fb987f428367e603ae90906bdba004f"}],"src/jtag/drivers/cmsis_dap_usb_bulk.c":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"2e0bc47a05436e1353519620c9f015434d33e8d8","unresolved":true,"context_lines":[{"line_number":604,"context_line":"\t\tif (!bdata-\u003ecommand_transfers[i].buffer"},{"line_number":605,"context_line":"\t\t\t|| !bdata-\u003eresponse_transfers[i].buffer) {"},{"line_number":606,"context_line":"\t\t\tLOG_ERROR(\"unable to allocate CMSIS-DAP pending packet buffer\");"},{"line_number":607,"context_line":"\t\t\treturn ERROR_FAIL;"},{"line_number":608,"context_line":"\t\t}"},{"line_number":609,"context_line":"\t}"},{"line_number":610,"context_line":"\treturn ERROR_OK;"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"55e09f21_8b0f8585","line":607,"updated":"2023-12-29 16:06:26.000000000","message":"memory leak if the allocation of only one of the two buffer succeed.\nYou should call oocd_libusb_dev_mem_free() on both.\n\nCalling oocd_libusb_dev_mem_free() with a NULL pointer, in the option heap free() will work; not sure about the libusb_dev_mem_free() option.\nThe current libusb implementation for Linux only calls munmap() that should be ok with NULL as from munmap man page:\n\u003e If there are no mappings in the specified address range, then munmap() has no effect\n\nMaybe to prevent any future issue with other OS, the oocd_wrapper should handle the case of NULL pointer and return immediately.","commit_id":"f1e185478fb987f428367e603ae90906bdba004f"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"b0d6e8d02c7375650beb3702c40f719e79855d6f","unresolved":false,"context_lines":[{"line_number":604,"context_line":"\t\tif (!bdata-\u003ecommand_transfers[i].buffer"},{"line_number":605,"context_line":"\t\t\t|| !bdata-\u003eresponse_transfers[i].buffer) {"},{"line_number":606,"context_line":"\t\t\tLOG_ERROR(\"unable to allocate CMSIS-DAP pending packet buffer\");"},{"line_number":607,"context_line":"\t\t\treturn ERROR_FAIL;"},{"line_number":608,"context_line":"\t\t}"},{"line_number":609,"context_line":"\t}"},{"line_number":610,"context_line":"\treturn ERROR_OK;"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"a012063e_1ad996e1","line":607,"in_reply_to":"55e09f21_8b0f8585","updated":"2023-12-29 19:34:24.000000000","message":"\u003e memory leak if the allocation of only one of the two buffer succeed.\n\u003e You should call oocd_libusb_dev_mem_free() on both.\n \nI don\u0027t think there could be any memory leak.\nBe aware that not only two memory chunks are allocated, allocation is iterated over `i` to MAX_PENDING_REQUESTS.\nIf any allocation fails cmsis_dap_usb_close() is called immediately otherwise at exit: cmsis_dap_usb_free() gets always called and frees all allocated chunks.\n\n\u003e Calling oocd_libusb_dev_mem_free() with a NULL pointer,\n\nCould you look to\nhttps://review.openocd.org/c/openocd/+/8059/2/src/jtag/drivers/libusb_helper.c#420\n\nBTW It was your idea to remove NULLs from comparisons.","commit_id":"f1e185478fb987f428367e603ae90906bdba004f"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"5e857da216effb6c2376b8467f5e0ccd3a3f4e22","unresolved":false,"context_lines":[{"line_number":604,"context_line":"\t\tif (!bdata-\u003ecommand_transfers[i].buffer"},{"line_number":605,"context_line":"\t\t\t|| !bdata-\u003eresponse_transfers[i].buffer) {"},{"line_number":606,"context_line":"\t\t\tLOG_ERROR(\"unable to allocate CMSIS-DAP pending packet buffer\");"},{"line_number":607,"context_line":"\t\t\treturn ERROR_FAIL;"},{"line_number":608,"context_line":"\t\t}"},{"line_number":609,"context_line":"\t}"},{"line_number":610,"context_line":"\treturn ERROR_OK;"}],"source_content_type":"text/x-csrc","patch_set":5,"id":"bfc2de28_af15c3be","line":607,"in_reply_to":"a012063e_1ad996e1","updated":"2023-12-29 21:15:18.000000000","message":"You are right! no leak in this file, OK.\nThe functions cmsis_dap_usb_alloc() and cmsis_dap_usb_free() are referenced also through\ncmsis_dap_handle-\u003ebackend-\u003epacket_buffer_alloc()\ncmsis_dap_handle-\u003ebackend-\u003epacket_buffer_free()\nin src/jtag/drivers/cmsis_dap.c\nThere there is a memory leak when in cmsis_dap_init() the alloc fails","commit_id":"f1e185478fb987f428367e603ae90906bdba004f"}]}