)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"801006a1180708e23ee49e604045387cccd4b74b","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"dd8995ee_e8ffd2da","updated":"2026-06-24 07:59:31.000000000","message":"Hi Marc, \n\nthank you for preparing the change. Overall I like the concept very much.\n\nMy only comment is about the warnings in `jtag_vpi_init()`: I would prefer them to be worded differently and to use different code around them for clarity.\n\nCan you please check out my proposal?","commit_id":"ffa2943575c8f0a3b459b7474a03a04805ca4e88"}],"src/jtag/drivers/jtag_vpi.c":[{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"801006a1180708e23ee49e604045387cccd4b74b","unresolved":true,"context_lines":[{"line_number":536,"context_line":"\tconst char *address \u003d remote-\u003eaddress;"},{"line_number":537,"context_line":"\tuint16_t port \u003d remote-\u003eport;"},{"line_number":538,"context_line":""},{"line_number":539,"context_line":"\tif (remote-\u003eaddress \u0026\u0026 remote-\u003etype \u003d\u003d ADAPTER_REMOTE_TYPE_UNIX) {"},{"line_number":540,"context_line":"\t\tLOG_ERROR(\"Unix domain socket is not supported by this adapter\");"},{"line_number":541,"context_line":"\t\treturn ERROR_FAIL;"},{"line_number":542,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":2,"id":"15b11edc_ddae7117","line":539,"updated":"2026-06-24 07:59:31.000000000","message":"I believe the warnings should be updated so that they are very clear to the user and to the point.\n\nAlso I would prefer to clean up the code around so that it is very clear for future readers.\n\n- For code clarity, I recommend to rename `server_address` and `server_port` to `legacy_cmd_address` and `legacy_cmd_port`. \n\n- The warning `use \u0027adapter remote\u0027 instead of jtag_vpi set_xxx\u0027` should only be displayed if the user actually used the old commands.\n\n- Also it is more clear and natural to say \"instead of\" instead of \"not\".\n\n- I would promote the print about default port from debug to info since it sounds like important information for the user.\n\n- I recommend to clean up the code so that the readers will quickly understand that there are three possible sources for the address and port (new commands, old commands and the default).\n\nPlease, check out my proposal below. Would you like it?\n\n\n```\n// At the top of the file:\n\nstatic bool legacy_cmd_port_used;\nstatic uint16_t legacy_cmd_port \u003d DEFAULT_SERVER_PORT;\nstatic bool legacy_cmd_address_used;\nstatic char *legacy_cmd_address;\n\n// ...\n// ...\n//\tIn jtag_vpi_init():\n\n\tif (remote-\u003eaddress \u0026\u0026 remote-\u003etype \u003d\u003d ADAPTER_REMOTE_TYPE_UNIX) {\n\t\tLOG_ERROR(\"Unix domain socket is not supported by this adapter\");\n\t\treturn ERROR_FAIL;\n\t}\n\n\tif (legacy_cmd_address_used || legacy_cmd_port_used) {\n\t\tLOG_WARNING(\"DEPRECATED! use \u0027adapter remote\u0027 instead of \u0027jtag_vpi set_address\u0027 and \u0027jtag_vpi set_port\u0027\");\n\t}\n\n\t// Obtain host/address\n\tif (remote-\u003eaddress) {\n\t\t// Take from the preferred command\n\t\taddress \u003d remote-\u003eaddress;\n\t} else if (legacy_cmd_address_used) {\n\t\t// Take from the legacy command\n\t\tassert(legacy_cmd_address);\n\t\taddress \u003d legacy_cmd_address;\n\t} else {\n\t\t// Take the default + warn the user\n\t\tLOG_WARNING(\"DEPRECATED! Please specify the jtag_vpi server address (and port) explicitly using \u0027adapter remote\u0027\");\n\t\tLOG_WARNING(\"\u0027localhost\u0027 will be used as the default\");\n\t\tlegacy_cmd_server_address \u003d strdup(DEFAULT_SERVER_ADDRESS);\n\t\taddress \u003d legacy_cmd_server_address;\n\t}\n\n\t// Obtain port\n\tif (remote-\u003eport) {\n\t\t// Take from the preferred command\n\t\tport \u003d remote-\u003eport;\n\t} else if (legacy_cmd_port_used) {\n\t\t// Take from the legacy command\n\t\tport \u003d legacy_cmd_port;\n\t} else {\n\t\t// Take the default \u0026 inform the user\n\t\tport \u003d DEFAULT_SERVER_PORT;\n\t\tLOG_INFO(\"Server port not specified for jtag_vpi. Using default port %\" PRIu16, port);\t\t\n\t}\n```","commit_id":"ffa2943575c8f0a3b459b7474a03a04805ca4e88"}]}