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.
> --- 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.
> --- 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
