{"thread":{"id":"49587","subject":"Recommendations for adding strerror() in log statament","startedAt":"2018-10-17T08:32:35Z","lastAt":"2018-10-17T08:32:35Z","messageCount":1,"participants":["牛旭"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"360730","messageId":"4413c65c.101fa.16681291a04.Coremail.niuxu16@nudt.edu.cn","threadId":"49587","inReplyTo":null,"subject":"Recommendations for adding strerror() in log statament","fromName":"牛旭","fromEmail":"niuxu16@nudt.edu.cn","sentAt":"2018-10-17T08:34:06Z","receivedAt":"2018-10-17T08:32:35Z","isPatch":false,"sender":{"key":"niuxu16@nudt.edu.cn","avatar":null},"body":"Hi,\nOur team works on enhance logging practices by learning from historical log revisions in evolution.\nAnd we find 2 patches that add strerror() to the log statement which is printed when the return value of commit_lock_file() is smaller than 0.\n\nWhile applying this rule to git-2.14.2, we find 3 missed spots. \nWe suggest to add strerror() or just use error_errno() instead of error().\n\nHere are the missed spots:\n1) Line 307 in file git-2.14.2/sequencer.c:\nstatic int write_message(const void *buf, size_t len, const char *filename,\n\t\t\t int append_eol)\n{\n\t...\n\tif (append_eol && write(msg_fd, \"\\n\", 1) < 0) {\n\t\trollback_lock_file(&msg_file);\n\t\treturn error_errno(_(\"could not write eol to '%s'\"), filename);\n\t}\n\tif (commit_lock_file(&msg_file) < 0) {\n\t\trollback_lock_file(&msg_file);\n\t\treturn error(_(\"failed to finalize '%s'.\"), filename);\n\t}\n\n2) Line 1582 in file git-2.14.2/sequencer.c:\nstatic int save_head(const char *head)\n{\n\t...\n\tstrbuf_addf(&buf, \"%s\\n\", head);\n\tif (write_in_full(fd, buf.buf, buf.len) < 0) {\n\t\trollback_lock_file(&head_lock);\n\t\treturn error_errno(_(\"could not write to '%s'\"),\n\t\t\t\t   git_path_head_file());\n\t}\n\tif (commit_lock_file(&head_lock) < 0) {\n\t\trollback_lock_file(&head_lock);\n\t\treturn error(_(\"failed to finalize '%s'.\"), git_path_head_file());\n\t}\n\n3) Line 1706 in file git-2.14.2/sequencer.c:\nstatic int save_todo(struct todo_list *todo_list, struct replay_opts *opts)\n{\n\t...\n\tif (write_in_full(fd, todo_list->buf.buf + offset,\n\t\t\ttodo_list->buf.len - offset) < 0)\n\t\treturn error_errno(_(\"could not write to '%s'\"), todo_path);\n\tif (commit_lock_file(&todo_lock) < 0)\n\t\treturn error(_(\"failed to finalize '%s'.\"), todo_path);\n\n\nFollowing are the 2 patches that support our opinion:\n1) Line 2147 in file git-2.6.4/config.c:\n \tif (commit_lock_file(lock) < 0) {\n-\t\terror(\"could not commit config file %s\", config_filename);\n+\t\terror(\"could not write config file %s: %s\", config_filename,\n+\t\t      strerror(errno));\n \t\tret = CONFIG_NO_WRITE;\n \t\tlock = NULL;\n \t\tgoto out_free;\n \t}\n\n2) Line 2333 in file git-2.6.4/config.c:\n unlock_and_out:\n \tif (commit_lock_file(lock) < 0)\n-\t\tret = error(\"could not commit config file %s\", config_filename);\n+\t\tret = error(\"could not write config file %s: %s\",\n+\t\t\t    config_filename, strerror(errno));\n out:\n \tfree(filename_buf);\n\nThanks for reading, we are looking forward to your reply about the correctness of our suggestion~\nMay you a good day ^^\n\nBest regards,\nXu\n\n"}]}