{"thread":{"id":"43413","subject":"Re: [PATCH] Seek back to current filepos when mmap()ing with NO_MMAP","startedAt":"2006-11-15T16:27:54Z","lastAt":"2006-11-15T19:06:01Z","messageCount":4,"participants":["Johannes Schindelin","Alex Riesen","Morten Welinder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"295215","messageId":"Pine.LNX.4.63.0611151727000.13772@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"43413","inReplyTo":null,"subject":"[PATCH] Seek back to current filepos when mmap()ing with NO_MMAP","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-11-15T16:27:54Z","receivedAt":"2006-11-15T16:27:54Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\n\"git-index-pack --fix-thin\" relies on mmap() not changing the current\nfile position (otherwise the pack will be corrupted when writing the\nfinal SHA1). Meet that expectation.\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tSorry for not catching that earlier. It only affects platforms\n\twhere you have to set NO_MMAP=YesPlease, anyway.\n\n compat/mmap.c | 6 ++++++\n 1 files changed, 6 insertions(+), 0 deletions(-)\n\ndiff --git a/compat/mmap.c b/compat/mmap.c\nindex 55cb120..a4d2e50 100644\n--- a/compat/mmap.c\n+++ b/compat/mmap.c\n@@ -7,6 +7,7 @@\n void *gitfakemmap(void *start, size_t length, int prot , int flags, int fd, off_t offset)\n {\n \tint n = 0;\n+\toff_t current_offset = lseek(fd, 0, SEEK_CUR);\n \n \tif (start != NULL || !(flags & MAP_PRIVATE))\n \t\tdie(\"Invalid usage of gitfakemmap.\");\n@@ -39,6 +40,11 @@ void *gitfakemmap(void *start, size_t le\n \t\tn += count;\n \t}\n \n+\tif (current_offset != lseek(fd, current_offset, SEEK_SET)) {\n+\t\terrno = EINVAL;\n+\t\treturn MAP_FAILED;\n+\t}\n+\n \treturn start;\n }\n \n-- \ndummy-dirty\n"},{"id":"297097","messageId":"118833cc0611151026q25fb6eeeid3a74e20498f00d@mail.gmail.com","threadId":"43413","inReplyTo":"Pine.LNX.4.63.0611151727000.13772@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Seek back to current filepos when mmap()ing with NO_MMAP","fromName":"Morten Welinder","fromEmail":"mwelinder@gmail.com","sentAt":"2006-11-15T18:26:56Z","receivedAt":"2006-11-15T18:26:56Z","isPatch":true,"sender":{"key":"mwelinder@gmail.com","avatar":null},"body":"Using xmalloc in there seems wrong. A plain malloc would do and the\nerror would be returned back to the mmap caller.\n\n"},{"id":"294658","messageId":"Pine.LNX.4.63.0611151932230.13772@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"43413","inReplyTo":"118833cc0611151026q25fb6eeeid3a74e20498f00d@mail.gmail.com","subject":"Re: [PATCH] Seek back to current filepos when mmap()ing with NO_MMAP","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-11-15T18:32:48Z","receivedAt":"2006-11-15T18:32:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 15 Nov 2006, Morten Welinder wrote:\n\n> Using xmalloc in there seems wrong. A plain malloc would do and the\n> error would be returned back to the mmap caller.\n\nI did not touch that code at all in this patch.\n\nCiao,\nDscho\n"},{"id":"295332","messageId":"81b0412b0611151106k45ee03ekd8a3a04173f618c3@mail.gmail.com","threadId":"43413","inReplyTo":"Pine.LNX.4.63.0611151727000.13772@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Seek back to current filepos when mmap()ing with NO_MMAP","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2006-11-15T19:06:01Z","receivedAt":"2006-11-15T19:06:01Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On 11/15/06, Johannes Schindelin wrote:\n>\n> \"git-index-pack --fix-thin\" relies on mmap() not changing the current\n> file position (otherwise the pack will be corrupted when writing the\n> final SHA1). Meet that expectation.\n>\n\nThanks! I was wondering for some considerable time why the\nhell t5500-fetch-pack fails.\n\nBTW, I extended error handling in that mmap. Dunno why.\n\nSigned-off-by: Alex Riesen \n\n\ncommit 40ed0644ca8d250f716f49a5c3e027aa2c1d3167\nAuthor: Alex Riesen \nDate: Wed Nov 15 19:55:06 2006 +0100\n\n Seek back to current filepos when mmap()ing with NO_MMAP, eh improved\n \n \"git-index-pack --fix-thin\" relies on mmap() not changing the current\n file position (otherwise the pack will be corrupted when writing the\n final SHA1). Meet that expectation.\n \n Signed-off-by: Johannes Schindelin \n Signed-off-by: Alex Riesen \n\ndiff --git a/compat/mmap.c b/compat/mmap.c\nindex 55cb120..c6d5cfe 100644\n--- a/compat/mmap.c\n+++ b/compat/mmap.c\n@@ -6,7 +6,8 @@\n \n void *gitfakemmap(void *start, size_t length, int prot , int flags, int fd, off_t offset)\n {\n-\tint n = 0;\n+\tint n = 0, err = 0;\n+\toff_t current_offset = lseek(fd, 0, SEEK_CUR);\n \n \tif (start != NULL || !(flags & MAP_PRIVATE))\n \t\tdie(\"Invalid usage of gitfakemmap.\");\n@@ -18,8 +19,8 @@ void *gitfakemmap(void *start, size_t le\n \n \tstart = xmalloc(length);\n \tif (start == NULL) {\n-\t\terrno = ENOMEM;\n-\t\treturn MAP_FAILED;\n+\t\terr = ENOMEM;\n+\t\tgoto unseek;\n \t}\n \n \twhile (n < length) {\n@@ -31,14 +32,22 @@ void *gitfakemmap(void *start, size_t le\n \t\t}\n \n \t\tif (count < 0) {\n-\t\t\tfree(start);\n-\t\t\terrno = EACCES;\n-\t\t\treturn MAP_FAILED;\n+\t\t\terr = EACCES;\n+\t\t\tgoto unseek;\n \t\t}\n \n \t\tn += count;\n \t}\n \n+unseek:\n+\tif (current_offset != lseek(fd, current_offset, SEEK_SET))\n+\t\terr = EINVAL;\n+\n+\tif (err) {\n+\t\tfree(start);\n+\t\tstart = MAP_FAILED;\n+\t\terrno = err;\n+\t}\n \treturn start;\n }\n \n"}]}