)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"ba8712f94c20b93c55577f723a55a6759862fa9a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"90660297_fc7eaff7","updated":"2024-01-13 18:59:04.000000000","message":"Thanks Evgeniy.\nI have one comment to further cleanup the command","commit_id":"42c299afadcd0bbe3dbf4af451e5759fa4cb0fc8"},{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"54699e204e54549a7f4ba51d288af73f3573d08b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"cf6ca7d4_7f8f8f6e","updated":"2024-01-15 17:25:59.000000000","message":"Thank you for the review! Can you please make a small clarification for me to be able to create a change to the documentation on writing command handlers?","commit_id":"8390053df6c3f98b666b1e6d70b25dbcfbe493e0"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"38195029b9cfc14c7dc9af33deef67b2abb00af7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"6511d701_c629407e","updated":"2024-02-01 00:46:14.000000000","message":"thanks","commit_id":"8390053df6c3f98b666b1e6d70b25dbcfbe493e0"}],"src/helper/log.c":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"ba8712f94c20b93c55577f723a55a6759862fa9a","unresolved":true,"context_lines":[{"line_number":221,"context_line":"\tif (CMD_ARGC \u003d\u003d 1 \u0026\u0026 strcmp(CMD_ARGV[0], \"default\") !\u003d 0) {"},{"line_number":222,"context_line":"\t\tfile \u003d fopen(CMD_ARGV[0], \"w\");"},{"line_number":223,"context_line":"\t\tif (!file) {"},{"line_number":224,"context_line":"\t\t\tLOG_ERROR(\"failed to open output log \u0027%s\u0027\", CMD_ARGV[0]);"},{"line_number":225,"context_line":"\t\t\treturn ERROR_FAIL;"},{"line_number":226,"context_line":"\t\t}"},{"line_number":227,"context_line":"\t\tcommand_print(CMD, \"set log_output to \\\"%s\\\"\", CMD_ARGV[0]);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"7b456479_aabd7afb","line":224,"updated":"2024-01-13 18:59:04.000000000","message":"this should go through command_print() too","commit_id":"42c299afadcd0bbe3dbf4af451e5759fa4cb0fc8"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"38195029b9cfc14c7dc9af33deef67b2abb00af7","unresolved":false,"context_lines":[{"line_number":221,"context_line":"\tif (CMD_ARGC \u003d\u003d 1 \u0026\u0026 strcmp(CMD_ARGV[0], \"default\") !\u003d 0) {"},{"line_number":222,"context_line":"\t\tfile \u003d fopen(CMD_ARGV[0], \"w\");"},{"line_number":223,"context_line":"\t\tif (!file) {"},{"line_number":224,"context_line":"\t\t\tLOG_ERROR(\"failed to open output log \u0027%s\u0027\", CMD_ARGV[0]);"},{"line_number":225,"context_line":"\t\t\treturn ERROR_FAIL;"},{"line_number":226,"context_line":"\t\t}"},{"line_number":227,"context_line":"\t\tcommand_print(CMD, \"set log_output to \\\"%s\\\"\", CMD_ARGV[0]);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"dd6699ae_38f05269","line":224,"in_reply_to":"4a597b9c_b74e858e","updated":"2024-02-01 00:46:14.000000000","message":"Yes, it should be documented. I think I have already reported it in gerrit few time and it\u0027s getting annoying. Documentation is the way to go.\nIf you want to start it, you are more than welcome.\nHere is a starting point. Maybe we can collect other contributions during code review.\n\nThere are LOG_XXX() and TCL commands output.\n\nEvery TCL command, implemented through .handler and COMMAND_HANDLER(), should send it\u0027s output to command_print(), so it can be reused in TCL, e.g.:\nset result [command arg1 arg2]\nor\necho [targets]\nWhen a command throws an ERROR_COMMAND_SYNTAX_ERROR, all it\u0027s previous output is erased and replaced by the output of \u0027usage command_name\u0027. So there is no need to put any command_print() before return ERROR_COMMAND_SYNTAX_ERROR, as it would be dropped. And no need for LOG_ERROR() to report the syntax, as it would be already printed by \u0027usage ..\u0027.\nThe other useful ERROR_COMMAND_xxx are:\nERROR_COMMAND_ARGUMENT_INVALID\nERROR_COMMAND_ARGUMENT_OVERFLOW\nERROR_COMMAND_ARGUMENT_UNDERFLOW\nthat should be used when the value of an argument is out-of-range or forbidden. I normally use ERROR_COMMAND_ARGUMENT_INVALID only.\nIf a command\u0027s sub-function returns an error, it should be propagated.\nThe generic ERROR_FAIL is used for almost all the other cases.\n\nInside OpenOCD there is some asynchronous code, not really part of a TCL command: jtag queue handling, target\u0027s polling, ...\nThis code cannot use command_print(), so LOG_XXX() is the only way to go.\n\nLOG_DEBUG() can be used in TCL commands, as it is for debugging OpenOCD and does not produce a command output.\nLOG_WARNING() can be used in TCL commands when the user has to be alerted of some specific condition.\n\nEverywhere on allocation failure from alloc(), calloc(), realloc(), strdup(), ..., it should be used LOG_ERROR(\"Out of memory\"), also in TCL commands. This in the hope that the text would be printed out before the OS kills OpenOCD.\n\nNo need to document what to do in .jim_handler commands, as I\u0027m converting all of them to .handler commands.\n\nSaid that, there are many TCL commands that don\u0027t use yet command_print(), or use it partially, or does not pass to sub-functions the cmd pointer to use command_print(). I have modified some, but we are far from a systematic code review.","commit_id":"42c299afadcd0bbe3dbf4af451e5759fa4cb0fc8"},{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"54699e204e54549a7f4ba51d288af73f3573d08b","unresolved":true,"context_lines":[{"line_number":221,"context_line":"\tif (CMD_ARGC \u003d\u003d 1 \u0026\u0026 strcmp(CMD_ARGV[0], \"default\") !\u003d 0) {"},{"line_number":222,"context_line":"\t\tfile \u003d fopen(CMD_ARGV[0], \"w\");"},{"line_number":223,"context_line":"\t\tif (!file) {"},{"line_number":224,"context_line":"\t\t\tLOG_ERROR(\"failed to open output log \u0027%s\u0027\", CMD_ARGV[0]);"},{"line_number":225,"context_line":"\t\t\treturn ERROR_FAIL;"},{"line_number":226,"context_line":"\t\t}"},{"line_number":227,"context_line":"\t\tcommand_print(CMD, \"set log_output to \\\"%s\\\"\", CMD_ARGV[0]);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"4a597b9c_b74e858e","line":224,"in_reply_to":"7b456479_aabd7afb","updated":"2024-01-15 17:25:59.000000000","message":"Addressed. Though can you please clarify where I can find the difference?\nI would like to update `doc/manual/primer/commands.txt` since I didn\u0027t find the information there and there are some issues with the example in this file, e.g. the use of `LOG_ERROR()` before returning `ERROR_COMMAND_SYNTAX_ERROR` is redundant, if I\u0027m correct.","commit_id":"42c299afadcd0bbe3dbf4af451e5759fa4cb0fc8"}]}