On Thu, Apr 19, 2012 at 3:18 PM, Nguyen Thai Ngoc Duy <pclouds@xxxxxxxxx> wrote:
> On Thu, Apr 19, 2012 at 7:58 PM, Erik Faye-Lund <kusmabite@xxxxxxxxx> wrote:
>> This approach has the problem that file-operations apart from pread
>> might (at least in theory) modify the position. To prevent that, we'd
>> either need to use the same locking-mechanism as the CRT use, or use
>> ReadFile with an OVERLAPPED struct, which allows us specify the offset
>> explicitly. The latter seems better to me, and should look something
>> like this (note: untested):
>
> Yeah. I read about ReadFile [1] but dismissed it when I got to async
> i/o mode. Reading again, sync i/o ReadFile with OVERLAPPED struct
> should work fine. It's not clear though if file offset is changed
> (pread man page says it does not change).
>
A quick test shows that it does not:
---8<---
$ cat test.c
#include <stdio.h>
#include <windows.h>
#include <errno.h>
#include <fcntl.h>
ssize_t mingw_pread(int fd, void *buf, size_t size, off_t offset)
{
OVERLAPPED overlapped = { 0 };
DWORD ret;
HANDLE fh = (HANDLE)_get_osfhandle(fd);
if (fh == INVALID_HANDLE_VALUE) {
errno = EBADF;
return -1;
}
overlapped.Offset = (DWORD)offset;
overlapped.OffsetHigh = (DWORD)(offset >> 32);
if (!ReadFile(fh, buf, size, &ret, &overlapped)) {
/* errno = err_win_to_posix(GetLastError()); */
return -1;
}
return ret;
}
int main(int argc, const char *argv[])
{
int i, fd = open(__FILE__, O_RDONLY);
for (i = 0; i < 2; ++i) {
char buf[11] = {0};
mingw_pread(fd, buf, 10, 0);
printf("buf = '%10s'\n", buf);
}
return 0;
}
$ gcc test.c -o test.exe && ./test.exe
test.c: In function 'mingw_pread':
test.c:18: warning: right shift count >= width of type
buf = '#include <'
buf = '#include <'
---8<---
So this looks fine to me.
> Also this approach deals with Windows only. There's still another
> NO_PREAD user, HP-UX something, and NO_PREAD comment mentions cygwin
> before v1.5.22. I personally don't care, just wanted to point out.
Yeah. Other platforms are still an issue. You didn't address those
either in your patch, even though it would be possible to modify it to
deal with them by checking the NO_PREAD and NO_PTHREADS defines.
But they would still have the problem with the file-pointer racing for
non-pread operations. Perhaps simply disabling threading is the better
choice for these?
--
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