On Tue, Jun 26, 2012 at 9:00 PM, Ramsay Jones
<ramsay@xxxxxxxxxxxxxxxxxxx> wrote:
> Hi Johannes,
>
> I had an "fix pread() on MinGW" item low on my todo list. As we have
> seen recently, Cygwin has the same problem. So, I decided to try and
> fix it up last night, since I had an idea that I thought would work.
>
> Several years ago, I read somewhere (a Microsoft Press publication of
> some sort) that, even when using *synchonous* I/O, you could pass an
> OVERLAPPED structure to ReadFile() and have it use the file offset in
> that structure, rather than the implicit stream pointer.
>
> So, I modified my "hacked up test program" from the other day to try
> it out; the diff looks like:
>
> -- >8 --
> diff --git a/test-pread.c b/test-pread.c
> index 61280cb..8a2b2ff 100644
> --- a/test-pread.c
> +++ b/test-pread.c
> @@ -1,5 +1,6 @@
> #include "git-compat-util.h"
> #include "thread-utils.h"
> +#include <windows.h>
>
> #define DATA_FILE "junk.data"
> #define MAX_DATA 256 * 1024
> @@ -16,6 +17,31 @@ struct thread_data {
>
> static struct thread_data t[NUM_THREADS+1];
>
> +ssize_t pread_(int fd, void *buf, size_t count, off_t offset)
> +{
> + OVERLAPPED o = {0};
> + HANDLE fh = (HANDLE)_get_osfhandle(fd);
> + uint64_t off = offset;
> + DWORD bytes;
> + BOOL ret;
> +
> + if (fh == INVALID_HANDLE_VALUE) {
> + errno = EBADF;
> + return -1;
> + }
> +
> + o.Offset = off & 0xffffffff;
> + o.OffsetHigh = (off >> 32) & 0xffffffff;
> +
> + ret = ReadFile(fh, buf, (DWORD)count, &bytes, &o);
> + if (!ret) {
> + errno = EIO;
> + return -1;
> + }
> +
> + return (ssize_t)bytes;
> +}
> +
> int create_data_file(void)
> {
> int i, fd = open(DATA_FILE, O_CREAT | O_TRUNC | O_WRONLY, 0600);
> @@ -55,7 +81,7 @@ void *pread_thread(void *data)
> ssize_t sz;
> d->n = d->n * 1103515245 + 12345;
> j = d->n % MAX_DATA;
> - sz = pread(d->fd, &rd, sizeof(int), j * sizeof(int));
> + sz = pread_(d->fd, &rd, sizeof(int), j * sizeof(int));
> if (sz < 0 || rd != j)
> d->fails++;
> d->cnt++;
>
> -- 8< --
>
> The result of running the updated program looks like:
>
> ramsay $ gcc -I. -o test-pread test-pread.c
> ramsay $ ./test-pread.exe
> 0: trials 524288, failed 262139
> 1: trials 500000, failed 0
> 2: trials 500000, failed 0
> 3: trials 500000, failed 0
> ramsay $
>
> So, the results are a little disappointing. :(
>
> Although ReadFile() is indeed using the OVERLAPPED structure to specify
> the read position, it is still updating the implicit stream position.
>
> So, this implementation does not faithfully reproduce the full semantics
> of pread(), since it updates the stream position (affecting other I/O
> calls which rely on the implicit file position. eg read()).
>
> [Note: this was tested on Windows XP SP3. Maybe Vista/Win7/Win8 would
> behave differently?]
>
> This implementation should be sufficient to fix the immediate problem with
> the threaded index-pack, but I'm not sure it would be a good idea to
> provide an *almost* compliant pread().
>
> I've attached the full test program below, just for completeness.
>
We've been through this before:
http://thread.gmane.org/CABPQNSZ6VdyoLcVUUJ4z5A2A7KGP8qBZAkzdx8zAtAs2mZN25w@xxxxxxxxxxxxxx
My worries are the same as yours: An approach like this looks a bit
too much like pread, and might make someone else assume it works the
same.
--
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