[PATCH] rerere: fix too-short initialization

This was caused by a typo in the sizeof parameter, and meant
we looked at uninitialized memory.  Caught by valgrind in
t2030.

Signed-off-by: Jeff King <redacted>
---
I'm running the whole test suite under valgrind for the current
'master'. This was the first hit, but it's very s-l-o-w, so there might
be more as the day progresses.

 rerere.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/rerere.c b/rerere.c
index a86d73d..d1d3e75 100644
--- a/rerere.c
+++ b/rerere.c

@@ -325,7 +325,7 @@ static int handle_cache(const char *path, unsigned char *sha1, const char *outpu
 	for (i = 0; i < 3; i++)
 		free(mmfile[i].ptr);
 
-	memset(&io, 0, sizeof(&io));
+	memset(&io, 0, sizeof(io));
 	io.io.getline = rerere_mem_getline;
 	if (output)
 		io.io.output = fopen(output, "w");

-- 
1.7.0.rc0.41.g538720

On Thu, Jan 28, 2010 at 09:52:16AM -0500, Jeff King wrote:

I'm running the whole test suite under valgrind for the current
'master'. This was the first hit, but it's very s-l-o-w, so there might
be more as the day progresses.

And here they are. The first two are actual bugs, the third silences a
false positive:

  [1/3]: fix memcpy of overlapping area
  [2/3]: fix off-by-one allocation error
  [3/3]: add shebang line to git-mergetool--lib.sh

It really has been a while since I've run the whole test suite under
valgrind. The breakage in 3/3 dates back to last April. The bug in 1/3
is also quite old (v1.5.0). I'm not sure why I didn't find it in
previous valgrind runs; perhaps it is only recent valgrinds that have
grown support for finding overlapping memcpy. The bug in 2/3 is new, and
has never been in a released version.

-Peff

Caught by valgrind in t5500, but it is pretty obvious from
reading the code that this is shifting elements of an array
to the left, which needs memmove.

Signed-off-by: Jeff King <redacted>
---
This was introduced in f53514b (allow deepening of a shallow repository,
2006-10-30), released as part of v1.5.0. So probably 'maint'-worthy.

 commit.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/commit.c b/commit.c
index 632061c..731191e 100644
--- a/commit.c
+++ b/commit.c

@@ -224,7 +224,7 @@ int unregister_shallow(const unsigned char *sha1)
 	if (pos < 0)
 		return -1;
 	if (pos + 1 < commit_graft_nr)
-		memcpy(commit_graft + pos, commit_graft + pos + 1,
+		memmove(commit_graft + pos, commit_graft + pos + 1,
 				sizeof(struct commit_graft *)
 				* (commit_graft_nr - pos - 1));
 	commit_graft_nr--;

-- 
1.7.0.rc0.41.g538720

Caught by valgrind in t5516. Reading the code shows we
malloc enough for our string, but not trailing NUL.

Signed-off-by: Jeff King <redacted>
---
This bug was introduced in f517f1f (builtin-push: add --delete as
syntactic sugar for :foo, 2009-12-30), not released yet but part of
1.7.0-rc0. So no need for a 'maint' fix.

An obvious alternative would be to convert it to strbuf (which could
also be used to clean up other non-buggy string generation earlier in
the function).

 builtin-push.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-push.c b/builtin-push.c
index 5df6608..5633f0a 100644
--- a/builtin-push.c
+++ b/builtin-push.c

@@ -52,7 +52,7 @@ static void set_refspecs(const char **refs, int nr)
 		} else if (deleterefs && !strchr(ref, ':')) {
 			char *delref;
 			int len = strlen(ref)+1;
-			delref = xmalloc(len);
+			delref = xmalloc(len+1);
 			strcpy(delref, ":");
 			strcat(delref, ref);
 			ref = delref;

-- 
1.7.0.rc0.41.g538720

Even though this script is expected to be sourced instead of
executed on its own, the #!/bin/sh line provides simple
documentation about what format the file is in.

In particular, the lack of such a line was confusing the
valgrind support of our test scripts, which assumed that any
executable without a #!-line should be intercepted and run
through valgrind. So during valgrind-enabled tests, any
script sourcing this file actually sourced the valgrind
interception script instead.

Signed-off-by: Jeff King <redacted>
---
The valgrind script could perhaps be a bit smarter instead, but checking
#!-lines is nice and simple, and this change makes other programs like
"file" happier, too.

This problem has been around since 21d0ba7 (difftool/mergetool: refactor
commands to use git-mergetool--lib, 2009-04-08), released in v1.6.3. But
since it is only about our internal tests, and even then only about
running them with valgrind enabled, I don't know if it is worth a fix on
'maint'.

 git-mergetool--lib.sh |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 5b62785..51dd0d6 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh

@@ -1,3 +1,4 @@
+#!/bin/sh
 # git-mergetool--lib is a library for common merge tool functions
 diff_mode() {
 	test "$TOOL_MODE" = diff

-- 
1.7.0.rc0.41.g538720

These scripts are expected to be sourced instead of executed on
their own.  Avoid some confusion by not marking them executable.

The executable bit was confusing the valgrind support of our test
scripts, which assumed that any executable without a #!-line
should be intercepted and run through valgrind.  So during
valgrind-enabled tests, any script sourcing this file actually
sourced the valgrind interception script instead.

Reported-by: Jeff King <redacted>
Signed-off-by: Jonathan Nieder <redacted>
---
Jeff King wrote:

This problem has been around since 21d0ba7 (difftool/mergetool: refactor
commands to use git-mergetool--lib, 2009-04-08), released in v1.6.3. But
since it is only about our internal tests, and even then only about
running them with valgrind enabled, I don't know if it is worth a fix on
'maint'.

It was also confusing dpkg-shlibdeps, so I recently came up with
this fix.  Both fixes seem like good changes to me, and both
could be applied.  Your fix has the virtue of being shorter,
hence safer.

 Makefile        |   24 ++++++++++++++++++------
 1 files changed, 18 insertions(+), 6 deletions(-)
 mode change 100755 => 100644 git-sh-setup.sh

diff --git a/Makefile b/Makefile
index fd7f51e..70dcc40 100644
--- a/Makefile
+++ b/Makefile

@@ -342,6 +342,7 @@ LIB_OBJS =
 PROGRAMS =
 SCRIPT_PERL =
 SCRIPT_SH =
+SCRIPT_LIB_SH =
 TEST_PROGRAMS =
 
 SCRIPT_SH += git-am.sh

@@ -353,7 +354,6 @@ SCRIPT_SH += git-merge-octopus.sh
 SCRIPT_SH += git-merge-one-file.sh
 SCRIPT_SH += git-merge-resolve.sh
 SCRIPT_SH += git-mergetool.sh
-SCRIPT_SH += git-mergetool--lib.sh
 SCRIPT_SH += git-notes.sh
 SCRIPT_SH += git-parse-remote.sh
 SCRIPT_SH += git-pull.sh

@@ -362,11 +362,13 @@ SCRIPT_SH += git-rebase--interactive.sh
 SCRIPT_SH += git-rebase.sh
 SCRIPT_SH += git-repack.sh
 SCRIPT_SH += git-request-pull.sh
-SCRIPT_SH += git-sh-setup.sh
 SCRIPT_SH += git-stash.sh
 SCRIPT_SH += git-submodule.sh
 SCRIPT_SH += git-web--browse.sh
 
+SCRIPT_LIB_SH += git-mergetool--lib.sh
+SCRIPT_LIB_SH += git-sh-setup.sh
+
 SCRIPT_PERL += git-add--interactive.perl
 SCRIPT_PERL += git-difftool.perl
 SCRIPT_PERL += git-archimport.perl

@@ -1428,7 +1430,7 @@ export TAR INSTALL DESTDIR SHELL_PATH
 
 SHELL = $(SHELL_PATH)
 
-all:: shell_compatibility_test $(ALL_PROGRAMS) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS
+all:: shell_compatibility_test $(ALL_PROGRAMS) $(SCRIPT_LIB_SH) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS
 ifneq (,$X)
 	$(QUIET_BUILT_IN)$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), test -d '$p' -o '$p' -ef '$p$X' || $(RM) '$p';)
 endif

@@ -1488,6 +1490,15 @@ $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh
 	chmod +x $@+ && \
 	mv $@+ $@
 
+$(patsubst %.sh,%,$(SCRIPT_LIB_SH)) : % : %.sh
+	$(QUIET_GEN)$(RM) $@ $@+ && \
+	sed -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \
+	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
+	    -e 's/@@NO_CURL@@/$(NO_CURL)/g' \
+	    -e $(BROKEN_PATH_FIX) \
+	    $@.sh >$@+ && \
+	mv $@+ $@
+
 ifndef NO_PERL
 $(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak
 

@@ -1792,6 +1803,7 @@ install: all
 	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'
 	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) -m 644 $(SCRIPT_LIB_SH) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 	$(INSTALL) git$X git-upload-pack$X git-receive-pack$X git-upload-archive$X git-shell$X git-cvsserver '$(DESTDIR_SQ)$(bindir_SQ)'
 	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
 ifndef NO_PERL

@@ -1901,7 +1913,7 @@ distclean: clean
 clean:
 	$(RM) *.o block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \
 		$(LIB_FILE) $(XDIFF_LIB)
-	$(RM) $(ALL_PROGRAMS) $(BUILT_INS) git$X
+	$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB_SH) $(BUILT_INS) git$X
 	$(RM) $(TEST_PROGRAMS)
 	$(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h TAGS tags cscope*
 	$(RM) -r autom4te.cache

@@ -1930,7 +1942,7 @@ endif
 ### Check documentation
 #
 check-docs::
-	@(for v in $(ALL_PROGRAMS) $(BUILT_INS) git gitk; \
+	@(for v in $(ALL_PROGRAMS) $(SCRIPT_LIB_SH) $(BUILT_INS) git gitk; \
 	do \
 		case "$$v" in \
 		git-merge-octopus | git-merge-ours | git-merge-recursive | \

@@ -1975,7 +1987,7 @@ check-docs::
 		documented,gittutorial-2 | \
 		sentinel,not,matching,is,ok ) continue ;; \
 		esac; \
-		case " $(ALL_PROGRAMS) $(BUILT_INS) git gitk " in \
+		case " $(ALL_PROGRAMS) $(SCRIPT_LIB_SH) $(BUILT_INS) git gitk " in \
 		*" $$cmd "*)	;; \
 		*) echo "removed but $$how: $$cmd" ;; \
 		esac; \

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
old mode 100755
new mode 100644
-- 
1.7.0.rc0