{"thread":{"id":"46006","subject":"[PATCH] Remove useless assignments","startedAt":"2017-05-20T06:03:54Z","lastAt":"2017-05-20T06:54:10Z","messageCount":2,"participants":["Дилян Палаузов","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"320296","messageId":"20170520055216.30470-1-git-dpa@aegee.org","threadId":"46006","inReplyTo":null,"subject":"[PATCH] Remove useless assignments","fromName":"Дилян Палаузов","fromEmail":"git-dpa@aegee.org","sentAt":"2017-05-20T05:52:16Z","receivedAt":"2017-05-20T06:03:54Z","isPatch":true,"sender":{"key":"git-dpa@aegee.org","avatar":"https://avatars.githubusercontent.com/u/4992947?v=4"},"body":"Signed-off-by: Дилян Палаузов <git-dpa@aegee.org>\n\ndiff --git a/archive.c b/archive.c\nindex 60b889198..e2534d186 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -503,7 +503,7 @@ int write_archive(int argc, const char **argv, const char *prefix,\n \tinit_tar_archiver();\n \tinit_zip_archiver();\n \n-\targc = parse_archive_args(argc, argv, &ar, &args, name_hint, remote);\n+\tparse_archive_args(argc, argv, &ar, &args, name_hint, remote);\n \tif (!startup_info->have_repository) {\n \t\t/*\n \t\t * We know this will die() with an error, so we could just\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 9f53f020d..fc1481273 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -211,7 +211,7 @@ static int edit_patch(int argc, const char **argv, const char *prefix)\n \tinit_revisions(&rev, prefix);\n \trev.diffopt.context = 7;\n \n-\targc = setup_revisions(argc, argv, &rev, NULL);\n+\tsetup_revisions(argc, argv, &rev, NULL);\n \trev.diffopt.output_format = DIFF_FORMAT_PATCH;\n \trev.diffopt.use_color = 0;\n \tDIFF_OPT_SET(&rev.diffopt, IGNORE_DIRTY_SUBMODULES);\ndiff --git a/builtin/help.c b/builtin/help.c\nindex 49f7a07f8..3d24b084e 100644\n--- a/builtin/help.c\n+++ b/builtin/help.c\n@@ -453,7 +453,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)\n \tint nongit;\n \tenum help_format parsed_help_format;\n \n-\targc = parse_options(argc, argv, prefix, builtin_help_options,\n+\tparse_options(argc, argv, prefix, builtin_help_options,\n \t\t\tbuiltin_help_usage, 0);\n \tparsed_help_format = help_format;\n \ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 703827f00..337efef6f 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -795,7 +795,7 @@ static int merge_trivial(struct commit *head, struct commit_list *remoteheads)\n \twrite_tree_trivial(&result_tree);\n \tprintf(_(\"Wonderful.\\n\"));\n \tpptr = commit_list_append(head, pptr);\n-\tpptr = commit_list_append(remoteheads->item, pptr);\n+\tcommit_list_append(remoteheads->item, pptr);\n \tprepare_to_commit(remoteheads);\n \tif (commit_tree(merge_msg.buf, merge_msg.len, result_tree.hash, parents,\n \t\t\tresult_commit.hash, NULL, sign_commit))\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 7b891471c..6ec753383 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -1015,7 +1015,7 @@ int cmd_notes(int argc, const char **argv, const char *prefix)\n \telse if (!strcmp(argv[0], \"get-ref\"))\n \t\tresult = get_ref(argc, argv, prefix);\n \telse {\n-\t\tresult = error(_(\"unknown subcommand: %s\"), argv[0]);\n+\t\terror(_(\"unknown subcommand: %s\"), argv[0]);\n \t\tusage_with_options(git_notes_usage, options);\n \t}\n \ndiff --git a/builtin/prune-packed.c b/builtin/prune-packed.c\nindex c026299e7..73f424d9f 100644\n--- a/builtin/prune-packed.c\n+++ b/builtin/prune-packed.c\n@@ -59,7 +59,7 @@ int cmd_prune_packed(int argc, const char **argv, const char *prefix)\n \t\tOPT_END()\n \t};\n \n-\targc = parse_options(argc, argv, prefix, prune_packed_options,\n+\tparse_options(argc, argv, prefix, prune_packed_options,\n \t\t\t     prune_packed_usage, 0);\n \n \tprune_packed_objects(opts);\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 0bb36d584..0fa779103 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -498,7 +498,7 @@ static const char *check_nonce(const char *buf, size_t len)\n \tchar *nonce = find_header(buf, len, \"nonce\");\n \ttimestamp_t stamp, ostamp;\n \tchar *bohmac, *expect = NULL;\n-\tconst char *retval = NONCE_BAD;\n+\tconst char *retval;\n \n \tif (!nonce) {\n \t\tretval = NONCE_MISSING;\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex fc3b906c4..f547efd8d 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -291,7 +291,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \n \tgit_config(git_default_config, NULL);\n \n-\targc = parse_options(argc, argv, prefix, options, git_reset_usage,\n+\tparse_options(argc, argv, prefix, options, git_reset_usage,\n \t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n \tparse_args(&pathspec, argv, prefix, patch_mode, &rev);\n \ndiff --git a/diffcore-rename.c b/diffcore-rename.c\nindex f7444c86b..632e19089 100644\n--- a/diffcore-rename.c\n+++ b/diffcore-rename.c\n@@ -580,7 +580,7 @@ void diffcore_rename(struct diff_options *options)\n \n \trename_count += find_renames(mx, dst_cnt, minimum_score, 0);\n \tif (detect_rename == DIFF_DETECT_COPY)\n-\t\trename_count += find_renames(mx, dst_cnt, minimum_score, 1);\n+\t\tfind_renames(mx, dst_cnt, minimum_score, 1);\n \tfree(mx);\n \n  cleanup:\ndiff --git a/fast-import.c b/fast-import.c\nindex cf58f875b..0369363b2 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -2054,7 +2054,7 @@ static int validate_raw_date(const char *src, struct strbuf *result)\n \n \terrno = 0;\n \n-\tnum = strtoul(src, &endp, 10);\n+\tstrtoul(src, &endp, 10);\n \t/* NEEDSWORK: perhaps check for reasonable values? */\n \tif (errno || endp == src || *endp != ' ')\n \t\treturn -1;\ndiff --git a/fsck.c b/fsck.c\nindex d589341cd..35d421c08 100644\n--- a/fsck.c\n+++ b/fsck.c\n@@ -703,7 +703,6 @@ static int fsck_ident(const char **ident, struct object *obj, struct fsck_option\n \t    !isdigit(p[4]) ||\n \t    (p[5] != '\\n'))\n \t\treturn report(options, obj, FSCK_MSG_BAD_TIMEZONE, \"invalid author/committer line - bad time zone\");\n-\tp += 6;\n \treturn 0;\n }\n \ndiff --git a/ref-filter.c b/ref-filter.c\nindex 1fc5e9970..2d06b98ce 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1634,10 +1634,6 @@ static int match_name_as_path(const struct ref_filter *filter, const char *refna\n {\n \tconst char **pattern = filter->name_patterns;\n \tint namelen = strlen(refname);\n-\tunsigned flags = WM_PATHNAME;\n-\n-\tif (filter->ignore_case)\n-\t\tflags |= WM_CASEFOLD;\n \n \tfor (; *pattern; pattern++) {\n \t\tconst char *p = *pattern;\ndiff --git a/t/helper/test-lazy-init-name-hash.c b/t/helper/test-lazy-init-name-hash.c\nindex 6368a8934..29a16a61b 100644\n--- a/t/helper/test-lazy-init-name-hash.c\n+++ b/t/helper/test-lazy-init-name-hash.c\n@@ -209,7 +209,7 @@ int cmd_main(int argc, const char **argv)\n \n \tprefix = setup_git_directory();\n \n-\targc = parse_options(argc, argv, prefix, options, usage, 0);\n+\tparse_options(argc, argv, prefix, options, usage, 0);\n \n \t/*\n \t * istate->dir_hash is only created when ignore_case is set.\ndiff --git a/wrapper.c b/wrapper.c\nindex d83741770..ad7b63a0f 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -485,7 +485,7 @@ int git_mkstemps_mode(char *pattern, int suffix_len, int mode)\n \t\ttemplate[2] = letters[v % num_letters]; v /= num_letters;\n \t\ttemplate[3] = letters[v % num_letters]; v /= num_letters;\n \t\ttemplate[4] = letters[v % num_letters]; v /= num_letters;\n-\t\ttemplate[5] = letters[v % num_letters]; v /= num_letters;\n+\t\ttemplate[5] = letters[v % num_letters];\n \n \t\tfd = open(pattern, O_CREAT | O_EXCL | O_RDWR, mode);\n \t\tif (fd >= 0)\n-- \n2.12.3\n\n"},{"id":"320301","messageId":"20170520065401.oqavzqh4szxoyqvl@sigill.intra.peff.net","threadId":"46006","inReplyTo":"20170520055216.30470-1-git-dpa@aegee.org","subject":"Re: [PATCH] Remove useless assignments","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-05-20T06:54:01Z","receivedAt":"2017-05-20T06:54:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, May 20, 2017 at 05:52:16AM +0000, Дилян Палаузов wrote:\n\n> Signed-off-by: Дилян Палаузов <git-dpa@aegee.org>\n\nI assume this is just going through the results of clang's static\nanalyzer and quieting it. I think most of these are not the right fix,\nthough, as I discussed in\n\n  http://public-inbox.org/git/20170225223146.ixubnwqkfol5q2gn@sigill.intra.peff.net/\n\nFor instance:\n\n> diff --git a/archive.c b/archive.c\n> index 60b889198..e2534d186 100644\n> --- a/archive.c\n> +++ b/archive.c\n> @@ -503,7 +503,7 @@ int write_archive(int argc, const char **argv, const char *prefix,\n>  \tinit_tar_archiver();\n>  \tinit_zip_archiver();\n>  \n> -\targc = parse_archive_args(argc, argv, &ar, &args, name_hint, remote);\n> +\tparse_archive_args(argc, argv, &ar, &args, name_hint, remote);\n>  \tif (!startup_info->have_repository) {\n>  \t\t/*\n>  \t\t * We know this will die() with an error, so we could just\n\nIt's true that nobody accesses argc after this, so the assignment is\ndead. But if we _don't_ assign to argc, we're leaving a maintenance trap\nfor somebody who later does want to access it. Its value must match that\nwhat is in argv, and after your patch that invariant no longer holds.\n\nDitto for all of the argc hunks in this patch.\n\n> diff --git a/builtin/merge.c b/builtin/merge.c\n> index 703827f00..337efef6f 100644\n> --- a/builtin/merge.c\n> +++ b/builtin/merge.c\n> @@ -795,7 +795,7 @@ static int merge_trivial(struct commit *head, struct commit_list *remoteheads)\n>  \twrite_tree_trivial(&result_tree);\n>  \tprintf(_(\"Wonderful.\\n\"));\n>  \tpptr = commit_list_append(head, pptr);\n> -\tpptr = commit_list_append(remoteheads->item, pptr);\n> +\tcommit_list_append(remoteheads->item, pptr);\n>  \tprepare_to_commit(remoteheads);\n>  \tif (commit_tree(merge_msg.buf, merge_msg.len, result_tree.hash, parents,\n>  \t\t\tresult_commit.hash, NULL, sign_commit))\n\nThis one is similar. The return value of the append function updates the\ntail pointer. Nobody appends to the list, so we never look at the tail\npointer again. But it has violated the tail-pointer invariant, and is\nsetting a trap for somebody who tries to append to the list later.\n\n> diff --git a/builtin/notes.c b/builtin/notes.c\n> index 7b891471c..6ec753383 100644\n> --- a/builtin/notes.c\n> +++ b/builtin/notes.c\n> @@ -1015,7 +1015,7 @@ int cmd_notes(int argc, const char **argv, const char *prefix)\n>  \telse if (!strcmp(argv[0], \"get-ref\"))\n>  \t\tresult = get_ref(argc, argv, prefix);\n>  \telse {\n> -\t\tresult = error(_(\"unknown subcommand: %s\"), argv[0]);\n> +\t\terror(_(\"unknown subcommand: %s\"), argv[0]);\n>  \t\tusage_with_options(git_notes_usage, options);\n>  \t}\n>  \n\nThis one actually seems reasonable (usage_with_options exits, so we\ndon't need to bother setting result).\n\n> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\n> index 0bb36d584..0fa779103 100644\n> --- a/builtin/receive-pack.c\n> +++ b/builtin/receive-pack.c\n> @@ -498,7 +498,7 @@ static const char *check_nonce(const char *buf, size_t len)\n>  \tchar *nonce = find_header(buf, len, \"nonce\");\n>  \ttimestamp_t stamp, ostamp;\n>  \tchar *bohmac, *expect = NULL;\n> -\tconst char *retval = NONCE_BAD;\n> +\tconst char *retval;\n>  \n>  \tif (!nonce) {\n>  \t\tretval = NONCE_MISSING;\n\nI have mixed feeling on this one. It's true that the NONCE_BAD value is\nnever used. But it's a defensive programming measure to set the default\nin case we add code paths that do not set retval. If we could trust the\ncompiler to tell us when the uninitialized value was used, that would\nlet us avoid that situation. But the maybe-uninitialized warnings have\nhistorically been one of the least trustworthy warnings we've seen (most\nof the compiler-warning workarounds we've done have been for that).\n\n> diff --git a/fsck.c b/fsck.c\n> index d589341cd..35d421c08 100644\n> --- a/fsck.c\n> +++ b/fsck.c\n> @@ -703,7 +703,6 @@ static int fsck_ident(const char **ident, struct object *obj, struct fsck_option\n>  \t    !isdigit(p[4]) ||\n>  \t    (p[5] != '\\n'))\n>  \t\treturn report(options, obj, FSCK_MSG_BAD_TIMEZONE, \"invalid author/committer line - bad time zone\");\n> -\tp += 6;\n>  \treturn 0;\n>  }\n\nAnother \"invariant\" one. Anybody adding more fsck checks would be\nsurprised when \"p\" is not updated past the last check.\n\n> diff --git a/ref-filter.c b/ref-filter.c\n> index 1fc5e9970..2d06b98ce 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -1634,10 +1634,6 @@ static int match_name_as_path(const struct ref_filter *filter, const char *refna\n>  {\n>  \tconst char **pattern = filter->name_patterns;\n>  \tint namelen = strlen(refname);\n> -\tunsigned flags = WM_PATHNAME;\n> -\n> -\tif (filter->ignore_case)\n> -\t\tflags |= WM_CASEFOLD;\n>  \n>  \tfor (; *pattern; pattern++) {\n>  \t\tconst char *p = *pattern;\n\nI think this one is a real bug, but your fix is going in the wrong\ndirection. Those lines were added by 3bb16a8bf (tag, branch,\nfor-each-ref: add --ignore-case for sorting and filtering, 2016-12-04),\nand it was probably a mistake that it did not update the call to\nwildmatch(). IOW, the fix is probably:\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 8f48e0dc4..f4306793b 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1533,7 +1533,7 @@ static int match_name_as_path(const struct ref_filter *filter, const char *refna\n \t\t     refname[plen] == '/' ||\n \t\t     p[plen-1] == '/'))\n \t\t\treturn 1;\n-\t\tif (!wildmatch(p, refname, WM_PATHNAME, NULL))\n+\t\tif (!wildmatch(p, refname, flags, NULL))\n \t\t\treturn 1;\n \t}\n \treturn 0;\n\n-Peff\n"}]}