Merge branch 'rs/cocci' into maint

gitster · gitster · commit c8fd2201756f · 2016-10-28T09:01:23.000-07:00
Code cleanup.

* rs/cocci:
  use strbuf_add_unique_abbrev() for adding short hashes, part 3
  remove unnecessary NULL check before free(3)
  coccicheck: make transformation for strbuf_addf(sb, "...") more precise
  use strbuf_add_unique_abbrev() for adding short hashes, part 2
  use strbuf_addstr() instead of strbuf_addf() with "%s", part 2
  gitignore: ignore output files of coccicheck make target
  use strbuf_addstr() for adding constant strings to a strbuf, part 2
  add coccicheck make target
  contrib/coccinelle: fix semantic patch for oid_to_hex_r()
diff --git a/Makefile b/Makefile
@@ -462,6 +462,7 @@ CURL_CONFIG = curl-config
 PTHREAD_LIBS = -lpthread
 PTHREAD_CFLAGS =
 GCOV = gcov
+SPATCH = spatch
 
 export TCL_PATH TCLTK_PATH
 
@@ -2308,6 +2309,18 @@ check: common-cmds.h
 		exit 1; \
 	fi
 
+C_SOURCES = $(patsubst %.o,%.c,$(C_OBJ))
+%.cocci.patch: %.cocci $(C_SOURCES)
+	@echo '    ' SPATCH $<; \
+	for f in $(C_SOURCES); do \
+		$(SPATCH) --sp-file $< $$f; \
+	done >$@ 2>$@.log; \
+	if test -s $@; \
+	then \
+		echo '    ' SPATCH result: $@; \
+	fi
+coccicheck: $(patsubst %.cocci,%.cocci.patch,$(wildcard contrib/coccinelle/*.cocci))
+
 ### Installation rules
 
 ifneq ($(filter /%,$(firstword $(template_dir))),)
@@ -2499,6 +2512,7 @@ clean: profile-clean coverage-clean
 	$(RM) -r $(GIT_TARNAME) .doc-tmp-dir
 	$(RM) $(GIT_TARNAME).tar.gz git-core_$(GIT_VERSION)-*.tar.gz
 	$(RM) $(htmldocs).tar.gz $(manpages).tar.gz
+	$(RM) contrib/coccinelle/*.cocci.patch*
 	$(MAKE) -C Documentation/ clean
 ifndef NO_PERL
 	$(MAKE) -C gitweb clean
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
@@ -395,7 +395,7 @@ static void shortlog(const char *name,
 
 	for (i = 0; i < subjects.nr; i++)
 		if (i >= limit)
-			strbuf_addf(out, "  ...\n");
+			strbuf_addstr(out, "  ...\n");
 		else
 			strbuf_addf(out, "  %s\n", subjects.items[i].string);
 
diff --git a/builtin/merge.c b/builtin/merge.c
@@ -940,7 +940,7 @@ static void write_merge_state(struct commit_list *remoteheads)
 
 	strbuf_reset(&buf);
 	if (fast_forward == FF_NO)
-		strbuf_addf(&buf, "no-ff");
+		strbuf_addstr(&buf, "no-ff");
 	write_file_buf(git_path_merge_mode(), buf.buf, buf.len);
 }
 
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
@@ -637,7 +637,7 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
 		if (suc->recursive_prefix)
 			strbuf_addf(&sb, "%s/%s", suc->recursive_prefix, ce->name);
 		else
-			strbuf_addf(&sb, "%s", ce->name);
+			strbuf_addstr(&sb, ce->name);
 		strbuf_addf(out, _("Skipping unmerged submodule %s"), sb.buf);
 		strbuf_addch(out, '\n');
 		goto cleanup;
@@ -749,8 +749,9 @@ static int update_clone_get_next_task(struct child_process *child,
 		ce = suc->failed_clones[index];
 		if (!prepare_to_clone_next_submodule(ce, child, suc, err)) {
 			suc->current ++;
-			strbuf_addf(err, "BUG: submodule considered for cloning,"
-				    "doesn't need cloning any more?\n");
+			strbuf_addstr(err, "BUG: submodule considered for "
+					   "cloning, doesn't need cloning "
+					   "any more?\n");
 			return 0;
 		}
 		p = xmalloc(sizeof(*p));
diff --git a/contrib/coccinelle/.gitignore b/contrib/coccinelle/.gitignore
@@ -0,0 +1 @@
+*.patch*
diff --git a/contrib/coccinelle/free.cocci b/contrib/coccinelle/free.cocci
@@ -0,0 +1,5 @@
+@@
+expression E;
+@@
+- if (E)
+  free(E);
diff --git a/contrib/coccinelle/object_id.cocci b/contrib/coccinelle/object_id.cocci
@@ -23,16 +23,16 @@ expression E1;
 + oid_to_hex(E1)
 
 @@
-expression E1;
+expression E1, E2;
 @@
-- sha1_to_hex_r(E1.hash)
-+ oid_to_hex_r(&E1)
+- sha1_to_hex_r(E1, E2.hash)
++ oid_to_hex_r(E1, &E2)
 
 @@
-expression E1;
+expression E1, E2;
 @@
-- sha1_to_hex_r(E1->hash)
-+ oid_to_hex_r(E1)
+- sha1_to_hex_r(E1, E2->hash)
++ oid_to_hex_r(E1, E2)
 
 @@
 expression E1;
diff --git a/contrib/coccinelle/strbuf.cocci b/contrib/coccinelle/strbuf.cocci
@@ -0,0 +1,40 @@
+@ strbuf_addf_with_format_only @
+expression E;
+constant fmt;
+@@
+  strbuf_addf(E,
+(
+  fmt
+|
+  _(fmt)
+)
+  );
+
+@ script:python @
+fmt << strbuf_addf_with_format_only.fmt;
+@@
+cocci.include_match("%" not in fmt)
+
+@ extends strbuf_addf_with_format_only @
+@@
+- strbuf_addf
++ strbuf_addstr
+  (E,
+(
+  fmt
+|
+  _(fmt)
+)
+  );
+
+@@
+expression E1, E2;
+@@
+- strbuf_addf(E1, "%s", E2);
++ strbuf_addstr(E1, E2);
+
+@@
+expression E1, E2, E3;
+@@
+- strbuf_addstr(E1, find_unique_abbrev(E2, E3));
++ strbuf_add_unique_abbrev(E1, E2, E3);
diff --git a/diff.c b/diff.c
@@ -3076,7 +3076,7 @@ static void fill_metainfo(struct strbuf *msg,
 		}
 		strbuf_addf(msg, "%s%sindex %s..", line_prefix, set,
 			    find_unique_abbrev(one->oid.hash, abbrev));
-		strbuf_addstr(msg, find_unique_abbrev(two->oid.hash, abbrev));
+		strbuf_add_unique_abbrev(msg, two->oid.hash, abbrev);
 		if (one->mode == two->mode)
 			strbuf_addf(msg, " %06o", one->mode);
 		strbuf_addf(msg, "%s\n", reset);
diff --git a/merge-recursive.c b/merge-recursive.c
@@ -202,11 +202,11 @@ static void output_commit_title(struct merge_options *o, struct commit *commit)
 		strbuf_addf(&o->obuf, "virtual %s\n",
 			merge_remote_util(commit)->name);
 	else {
-		strbuf_addf(&o->obuf, "%s ",
-			find_unique_abbrev(commit->object.oid.hash,
-				DEFAULT_ABBREV));
+		strbuf_add_unique_abbrev(&o->obuf, commit->object.oid.hash,
+					 DEFAULT_ABBREV);
+		strbuf_addch(&o->obuf, ' ');
 		if (parse_commit(commit) != 0)
-			strbuf_addf(&o->obuf, _("(bad commit)\n"));
+			strbuf_addstr(&o->obuf, _("(bad commit)\n"));
 		else {
 			const char *title;
 			const char *msg = get_commit_buffer(commit, NULL);
diff --git a/parse-options-cb.c b/parse-options-cb.c
@@ -199,8 +199,7 @@ int parse_opt_passthru(const struct option *opt, const char *arg, int unset)
 	if (recreate_opt(&sb, opt, arg, unset) < 0)
 		return -1;
 
-	if (*opt_value)
-		free(*opt_value);
+	free(*opt_value);
 
 	*opt_value = strbuf_detach(&sb, NULL);
 
diff --git a/pretty.c b/pretty.c
@@ -544,15 +544,13 @@ static void add_merge_info(const struct pretty_print_context *pp,
 	strbuf_addstr(sb, "Merge:");
 
 	while (parent) {
-		struct commit *p = parent->item;
-		const char *hex = NULL;
+		struct object_id *oidp = &parent->item->object.oid;
+		strbuf_addch(sb, ' ');
 		if (pp->abbrev)
-			hex = find_unique_abbrev(p->object.oid.hash, pp->abbrev);
-		if (!hex)
-			hex = oid_to_hex(&p->object.oid);
+			strbuf_add_unique_abbrev(sb, oidp->hash, pp->abbrev);
+		else
+			strbuf_addstr(sb, oid_to_hex(oidp));
 		parent = parent->next;
-
-		strbuf_addf(sb, " %s", hex);
 	}
 	strbuf_addch(sb, '\n');
 }
diff --git a/remote.c b/remote.c
@@ -2073,7 +2073,7 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
 			_("Your branch is based on '%s', but the upstream is gone.\n"),
 			base);
 		if (advice_status_hints)
-			strbuf_addf(sb,
+			strbuf_addstr(sb,
 				_("  (use \"git branch --unset-upstream\" to fixup)\n"));
 	} else if (!ours && !theirs) {
 		strbuf_addf(sb,
@@ -2086,7 +2086,7 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
 			   ours),
 			base, ours);
 		if (advice_status_hints)
-			strbuf_addf(sb,
+			strbuf_addstr(sb,
 				_("  (use \"git push\" to publish your local commits)\n"));
 	} else if (!ours) {
 		strbuf_addf(sb,
@@ -2097,7 +2097,7 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
 			   theirs),
 			base, theirs);
 		if (advice_status_hints)
-			strbuf_addf(sb,
+			strbuf_addstr(sb,
 				_("  (use \"git pull\" to update your local branch)\n"));
 	} else {
 		strbuf_addf(sb,
@@ -2110,7 +2110,7 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
 			   ours + theirs),
 			base, ours, theirs);
 		if (advice_status_hints)
-			strbuf_addf(sb,
+			strbuf_addstr(sb,
 				_("  (use \"git pull\" to merge the remote branch into yours)\n"));
 	}
 	free(base);
diff --git a/submodule.c b/submodule.c
@@ -370,11 +370,10 @@ void show_submodule_summary(FILE *f, const char *path,
 		return;
 	}
 
-	strbuf_addf(&sb, "%s%sSubmodule %s %s..", line_prefix, meta, path,
-			find_unique_abbrev(one, DEFAULT_ABBREV));
-	if (!fast_backward && !fast_forward)
-		strbuf_addch(&sb, '.');
-	strbuf_addf(&sb, "%s", find_unique_abbrev(two, DEFAULT_ABBREV));
+	strbuf_addf(&sb, "%s%sSubmodule %s ", line_prefix, meta, path);
+	strbuf_add_unique_abbrev(&sb, one, DEFAULT_ABBREV);
+	strbuf_addstr(&sb, (fast_backward || fast_forward) ? ".." : "...");
+	strbuf_add_unique_abbrev(&sb, two, DEFAULT_ABBREV);
 	if (message)
 		strbuf_addf(&sb, " %s%s\n", message, reset);
 	else
diff --git a/wt-status.c b/wt-status.c
@@ -367,11 +367,11 @@ static void wt_status_print_change_data(struct wt_status *s,
 		if (d->new_submodule_commits || d->dirty_submodule) {
 			strbuf_addstr(&extra, " (");
 			if (d->new_submodule_commits)
-				strbuf_addf(&extra, _("new commits, "));
+				strbuf_addstr(&extra, _("new commits, "));
 			if (d->dirty_submodule & DIRTY_SUBMODULE_MODIFIED)
-				strbuf_addf(&extra, _("modified content, "));
+				strbuf_addstr(&extra, _("modified content, "));
 			if (d->dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
-				strbuf_addf(&extra, _("untracked content, "));
+				strbuf_addstr(&extra, _("untracked content, "));
 			strbuf_setlen(&extra, extra.len - 2);
 			strbuf_addch(&extra, ')');
 		}
@@ -1053,17 +1053,17 @@ static void abbrev_sha1_in_line(struct strbuf *line)
 	split = strbuf_split_max(line, ' ', 3);
 	if (split[0] && split[1]) {
 		unsigned char sha1[20];
-		const char *abbrev;
 
 		/*
 		 * strbuf_split_max left a space. Trim it and re-add
 		 * it after abbreviation.
 		 */
 		strbuf_trim(split[1]);
 		if (!get_sha1(split[1]->buf, sha1)) {
-			abbrev = find_unique_abbrev(sha1, DEFAULT_ABBREV);
 			strbuf_reset(split[1]);
-			strbuf_addf(split[1], "%s ", abbrev);
+			strbuf_add_unique_abbrev(split[1], sha1,
+						 DEFAULT_ABBREV);
+			strbuf_addch(split[1], ' ');
 			strbuf_reset(line);
 			for (i = 0; split[i]; i++)
 				strbuf_addbuf(line, split[i]);
@@ -1286,10 +1286,8 @@ static char *get_branch(const struct worktree *wt, const char *path)
 	else if (starts_with(sb.buf, "refs/"))
 		;
 	else if (!get_sha1_hex(sb.buf, sha1)) {
-		const char *abbrev;
-		abbrev = find_unique_abbrev(sha1, DEFAULT_ABBREV);
 		strbuf_reset(&sb);
-		strbuf_addstr(&sb, abbrev);
+		strbuf_add_unique_abbrev(&sb, sha1, DEFAULT_ABBREV);
 	} else if (!strcmp(sb.buf, "detached HEAD")) /* rebase */
 		goto got_nothing;
 	else			/* bisect */
@@ -1326,8 +1324,7 @@ static int grab_1st_switch(unsigned char *osha1, unsigned char *nsha1,
 	if (!strcmp(cb->buf.buf, "HEAD")) {
 		/* HEAD is relative. Resolve it to the right reflog entry. */
 		strbuf_reset(&cb->buf);
-		strbuf_addstr(&cb->buf,
-			      find_unique_abbrev(nsha1, DEFAULT_ABBREV));
+		strbuf_add_unique_abbrev(&cb->buf, nsha1, DEFAULT_ABBREV);
 	}
 	return 1;
 }

Original file line number	Diff line number	Diff line change
`@@ -940,7 +940,7 @@ static void write_merge_state(struct commit_list *remoteheads)`
`940`	`940`
`941`	`941`	`strbuf_reset(&buf);`
`942`	`942`	`if (fast_forward == FF_NO)`
`943`		`- strbuf_addf(&buf, "no-ff");`
	`943`	`+ strbuf_addstr(&buf, "no-ff");`
`944`	`944`	`write_file_buf(git_path_merge_mode(), buf.buf, buf.len);`
`945`	`945`	`}`
`946`	`946`
-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +@@
 +expression E;
 +@@
 +- if (E)
 +  free(E);
Original file line number	Diff line number	Diff line change
`@@ -3076,7 +3076,7 @@ static void fill_metainfo(struct strbuf *msg,`
`3076`	`3076`	`}`
`3077`	`3077`	`strbuf_addf(msg, "%s%sindex %s..", line_prefix, set,`
`3078`	`3078`	`find_unique_abbrev(one->oid.hash, abbrev));`
`3079`		`- strbuf_addstr(msg, find_unique_abbrev(two->oid.hash, abbrev));`
	`3079`	`+ strbuf_add_unique_abbrev(msg, two->oid.hash, abbrev);`
`3080`	`3080`	`if (one->mode == two->mode)`
`3081`	`3081`	`strbuf_addf(msg, " %06o", one->mode);`
`3082`	`3082`	`strbuf_addf(msg, "%s\n", reset);`