threads / patch / 17544

patchapply: fix access to an uninitialized mode variable, found by valgrind

Subject: [PATCH] apply: fix access to an uninitialized mode variable, found by valgrind

## tl;dr

5 messages between Feb 4, 2009 and Feb 4, 2009. Diffs are folded; open one to read it.

replies: 4people: 4as markdown or json

Johannes Schindelin· Feb 4, 2009, 01:50 UTC · lore

When 'tpatch' was initialized successfully, st_mode was already taken from the previous diff. We should not try to override it with data from an lstat() that was never called.

This is a companion patch to 7a07841(git-apply: handle a patch that touches the same path more than once better).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin-apply.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Show changes to builtin-apply.c +1 −1
diff --git a/builtin-apply.c b/builtin-apply.c
index 1e7f917..f312798 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -2441,7 +2441,7 @@ static int check_preimage(struct patch *patch, struct cache_entry **ce, struct s
 		return error("%s: %s", old_name, strerror(errno));
 	}
 
-	if (!cached)
+	if (!cached && !tpatch)
 		st_mode = ce_mode_from_stat(*ce, st->st_mode);
 
 	if (patch->is_new < 0)

-- 
1.6.1.2.582.g3fdd5
Don Zickus· Feb 4, 2009, 14:36 UTC · re: Johannes Schindelin · lore

Re: [PATCH] apply: fix access to an uninitialized mode variable, found by valgrind

On Wed, Feb 04, 2009 at 02:50:15AM +0100, Johannes Schindelin wrote:
> When 'tpatch' was initialized successfully, st_mode was already taken
> from the previous diff.  We should not try to override it with data
> from an lstat() that was never called.
heh, oops.  thanks.
Acked-by:  Don Zickus <dzickus@redhat.com>
Jeff King· Feb 4, 2009, 15:31 UTC · re: Johannes Schindelin · lore

Re: [PATCH] apply: fix access to an uninitialized mode variable, found by valgrind

On Wed, Feb 04, 2009 at 02:50:15AM +0100, Johannes Schindelin wrote:
> When 'tpatch' was initialized successfully, st_mode was already taken
> from the previous diff.  We should not try to override it with data
> from an lstat() that was never called.
Yay. Valgrind For The Win, as the kids are saying these days.

Was this being triggered by t4121 and t4127? If so, then I think it solves the mystery valgrind errors I reported in the last paragraph here:

  http://article.gmane.org/gmane.comp.version-control.git/98898
-Peff
Johannes Schindelin· Feb 4, 2009, 16:44 UTC · re: Jeff King · lore

Re: [PATCH] apply: fix access to an uninitialized mode variable, found by valgrind

Hi,
On Wed, 4 Feb 2009, Jeff King wrote:
Show 7 quoted lines
> On Wed, Feb 04, 2009 at 02:50:15AM +0100, Johannes Schindelin wrote:
> 
> > When 'tpatch' was initialized successfully, st_mode was already taken
> > from the previous diff.  We should not try to override it with data
> > from an lstat() that was never called.
> 
> Yay. Valgrind For The Win, as the kids are saying these days.
Yes, indeed.
Show 5 quoted lines
> Was this being triggered by t4121 and t4127? If so, then I think it 
> solves the mystery valgrind errors I reported in the last paragraph 
> here:
> 
>   http://article.gmane.org/gmane.comp.version-control.git/98898
Yep, t4121 and t4127.  But I did not have time to follow the link.

Ciao, Dscho

← back to recent threads