[PATCH] Seek back to current filepos when mmap()ing with NO_MMAP

[PATCH] Seek back to current filepos when mmap()ing with NO_MMAP

[Johannes Schindelin]

"git-index-pack --fix-thin" relies on mmap() not changing the current
file position (otherwise the pack will be corrupted when writing the
final SHA1). Meet that expectation.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---

	Sorry for not catching that earlier. It only affects platforms
	where you have to set NO_MMAP=YesPlease, anyway.

 compat/mmap.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/compat/mmap.c b/compat/mmap.c
index 55cb120..a4d2e50 100644
--- a/compat/mmap.c
+++ b/compat/mmap.c
@@ -7,6 +7,7 @@
 void *gitfakemmap(void *start, size_t length, int prot , int flags, int fd, off_t offset)
 {
 	int n = 0;
+	off_t current_offset = lseek(fd, 0, SEEK_CUR);
 
 	if (start != NULL || !(flags & MAP_PRIVATE))
 		die("Invalid usage of gitfakemmap.");
@@ -39,6 +40,11 @@ void *gitfakemmap(void *start, size_t le
 		n += count;
 	}
 
+	if (current_offset != lseek(fd, current_offset, SEEK_SET)) {
+		errno = EINVAL;
+		return MAP_FAILED;
+	}
+
 	return start;
 }
 
-- 
dummy-dirty

Re: [PATCH] Seek back to current filepos when mmap()ing with NO_MMAP

[Morten Welinder]

Using xmalloc in there seems wrong.  A plain malloc would do and the
error would be returned back to the mmap caller.

Re: [PATCH] Seek back to current filepos when mmap()ing with NO_MMAP

[Johannes Schindelin]

Hi,

On Wed, 15 Nov 2006, Morten Welinder wrote:

> Using xmalloc in there seems wrong.  A plain malloc would do and the
> error would be returned back to the mmap caller.

I did not touch that code at all in this patch.

Ciao,
Dscho

Re: [PATCH] Seek back to current filepos when mmap()ing with NO_MMAP

[Alex Riesen]

[-- Attachment #1: Type: text/plain, Size: 450 bytes --]

On 11/15/06, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> "git-index-pack --fix-thin" relies on mmap() not changing the current
> file position (otherwise the pack will be corrupted when writing the
> final SHA1). Meet that expectation.
>

Thanks! I was wondering for some considerable time why the
hell t5500-fetch-pack fails.

BTW, I extended error handling in that mmap.  Dunno why.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: lseek-compatmmap.patch --]
[-- Type: text/x-diff; name="lseek-compatmmap.patch", Size: 1527 bytes --]

commit 40ed0644ca8d250f716f49a5c3e027aa2c1d3167
Author: Alex Riesen <raa.lkml@gmail.com>
Date:   Wed Nov 15 19:55:06 2006 +0100

    Seek back to current filepos when mmap()ing with NO_MMAP, eh improved
    
    "git-index-pack --fix-thin" relies on mmap() not changing the current
    file position (otherwise the pack will be corrupted when writing the
    final SHA1). Meet that expectation.
    
    Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
    Signed-off-by: Alex Riesen <raa.lkml@gmail.com>

diff --git a/compat/mmap.c b/compat/mmap.c
index 55cb120..c6d5cfe 100644
--- a/compat/mmap.c
+++ b/compat/mmap.c
@@ -6,7 +6,8 @@
 
 void *gitfakemmap(void *start, size_t length, int prot , int flags, int fd, off_t offset)
 {
-	int n = 0;
+	int n = 0, err = 0;
+	off_t current_offset = lseek(fd, 0, SEEK_CUR);
 
 	if (start != NULL || !(flags & MAP_PRIVATE))
 		die("Invalid usage of gitfakemmap.");
@@ -18,8 +19,8 @@ void *gitfakemmap(void *start, size_t le
 
 	start = xmalloc(length);
 	if (start == NULL) {
-		errno = ENOMEM;
-		return MAP_FAILED;
+		err = ENOMEM;
+		goto unseek;
 	}
 
 	while (n < length) {
@@ -31,14 +32,22 @@ void *gitfakemmap(void *start, size_t le
 		}
 
 		if (count < 0) {
-			free(start);
-			errno = EACCES;
-			return MAP_FAILED;
+			err = EACCES;
+			goto unseek;
 		}
 
 		n += count;
 	}
 
+unseek:
+	if (current_offset != lseek(fd, current_offset, SEEK_SET))
+		err = EINVAL;
+
+	if (err) {
+		free(start);
+		start = MAP_FAILED;
+		errno = err;
+	}
 	return start;
 }