Re: [PATCH] mmap implementation for mingw.
From: Johannes Sixt <hidden>
Date: 2016-06-15 22:45:40
Vasyl Vavrychuk schrieb:
Here is simple and restricted implementation of mmap using CreateFileMapping, MapViewOfFile.
Thanks. Sign-off? Did you notice any differences with this? Or is this change just because-we-can? It doesn't pass the test suite, for example t5301-sliding-window.sh fails.
quoted hunk ↗ jump to hunk
--- a/compat/mingw.c +++ b/compat/mingw.c@@ -994,3 +994,30 @@ void mingw_open_html(const char *unixpath) printf("Launching default browser to display HTML ...\n"); ShellExecute(NULL, "open", htmlpath, NULL, "\\", 0); } + +void *mingw_mmap(void *start, size_t length, int prot, int flags, int fd,off_t offset) +{ + HANDLE handle; + + if (start != NULL || !(flags & MAP_PRIVATE)) + die("Invalid usage of mingw_mmap");
I tend to use this idiom:
return errno = EINVAL,
error("Invalid usage of mingw_mmap");
+ if (offset % getpagesize() != 0)
+ die("Offset does not match the memory allocation granularity");This is dangerous. Because on MinGW getpagesize() is hard-coded to 0x1000. getpagesize() does not consult GetSystemInfo(). Just skip the check; MapViewOfFile() will report the error later anyway. Or better carefully compute a suitable offset and adjust the length accordingly.
+
+ handle = CreateFileMapping((HANDLE)_get_osfhandle(fd), NULL,
PAGE_WRITECOPY,
+ 0, 0, NULL);
+
+ if (handle != NULL) {
+ start = MapViewOfFile(handle, FILE_MAP_COPY, 0, offset,
length);
+ CloseHandle(handle);
+ }
+
+ return start;Upon failure you should return MAP_FAILED, not NULL.
quoted hunk ↗ jump to hunk
--- a/git-compat-util.h +++ b/git-compat-util.h@@ -175,10 +175,12 @@ static inline const char *skip_prefix(const char *str,const char *prefix) #define MAP_FAILED ((void*)-1) #endif +#ifndef __MINGW32__ #define mmap git_mmap #define munmap git_munmap extern void *git_mmap(void *start, size_t length, int prot, int flags, int fd, off_t offset); extern int git_munmap(void *start, size_t length); +#endif /* This value must be multiple of (pagesize * 2) */ #define DEFAULT_PACKED_GIT_WINDOW_SIZE (1 * 1024 * 1024)
This is inside #ifdef NO_MMAP ... #else section. Isn't that a bit strange? I.e. we say NO_MMAP, but then we do have mmap() now? -- Hannes