|
From: Keith M. <kei...@us...> - 2017-03-25 19:12:28
|
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 25/03/17 16:26, Eli Zaretskii wrote:
>> From: Keith Marshall
>> Date: Sat, 25 Mar 2017 15:58:17 +0000
>> On 25/03/17 12:34, Antonio Diaz Diaz wrote:
>>> The implementation I sent is indeed inadequate for general use. I was
>>> under the false impression that ReadFile/WriteFile were equivalent to
>>> pread/pwrite, but I have just been told that they are not. They modify
>>> the file position.
>>
>> Thanks. Without even looking at the code, I feared as much. On that
>> basis, they would not be acceptable for incorporation into MinGW, and
>> with that in mind, (that I will not use it), I did take a peek at your
>> code; it turns out to be equivalent to, (omitting error handling):
>>
>> ssize_t pread (int fd, void *buf, size_t count, __int64 offset)
>> {
>> _lseeki64 (fd, offset, SEEK_SET);
>> return read (fd, buf, count);
>> }
>>
>> ssize_t pwrite (int fd, const void *buf, size_t count, __int64 offset)
>> {
>> _lseeki64 (fd, offset, SEEK_SET);
>> return write (fd, buf, count);
>> }
>>
>> with no attempt whatsoever to either preserve, or to restore the file
>> pointer; (indeed, since these don't expose the ugliness of Microsoft's
>> lower level ReadFile() and WriteFile() APIs, I would favour such code
>> over that which you proposed).
>
> Why is that better than the original code proposal?
It is cleaner. It is better aligned with the standard it sets out to
implement. It better expresses what it actually does. It passes the
offset argument as-is, avoiding the gross ugliness of splitting into
two parts, to assign it to two separate fields within an OVERLAPPED
structure argument.
> That one at least didn't suffer from the race condition between the
> lseek and the following read/write call.
Really? How can we possibly know, without seeing Microsoft's kernel
source? MSDN warns that ReadFile() isn't thread safe, advising that
multi-threaded applications should protect I/O operations, which use
it, with mutex, or critical section synchronization protocols. Race
potential is present anyway, and just as likely between adjustment
of the file pointer and commencement of data transfer, as at any
other time within the scope of the ReadFile() call.
The foregoing applies equally to WriteFile().
> And where is the evidence that ReadFile/WriteFile indeed modify the
> file position? The MSDN docs says that the offset is updated, but
> says nothing about the file pointer.
It is trivially easy to demonstrate. If I link this:
#define _XOPEN_SOURCE 700
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <fcntl.h>
int global_fd;
const char *letters = "abcdefghijklmnopqrstuvwxyz";
int main()
{
char tmpname[] = "preadXXXXXX";
if( (global_fd = mkstemp( tmpname )) >= 0 )
{
ssize_t maxlen = strlen( letters );
write( global_fd, letters, maxlen );
close( global_fd );
if( (global_fd = open( tmpname, O_RDONLY )) >= 0 )
{
char buf[4];
ssize_t max_offset = maxlen - sizeof( buf );
pread( global_fd, buf, sizeof( buf ), 3 );
printf( "pread: %.4s\n", buf );
while( lseek( global_fd, 0, SEEK_CUR ) < max_offset )
{
read( global_fd, buf, sizeof( buf ) );
printf( "read: %.4s\n", buf );
}
close( global_fd );
}
}
remove( tmpname );
return 0;
}
with the OP's pread() implementation, and run it; I see:
$ WINEPATH=`pwd`/mingwrt ./a.exe ; rstty
pread: defg
read: hijk
read: lmno
read: pqrs
read: tuvw
(and identically the same when run for real, in a WinXP VM). If the
file pointer wasn't moved by the pread() call, (as it should not be),
the correct output would be:
$ WINEPATH=`pwd`/mingwrt ./a.exe ; rstty
pread: defg
read: abcd
read: efgh
read: ijkl
read: mnop
read: qrst
read: uvwx
- --
Regards,
Keith.
Public key available from keys.gnupg.net
Key fingerprint: C19E C018 1547 DE50 E1D4 8F53 C0AD 36C6 347E 5A3F
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.20 (GNU/Linux)
iQIcBAEBAgAGBQJY1sETAAoJEMCtNsY0flo/negP/ipw1WyKVtO4+FYitwXRkZzY
LZvVx0SC+z1nb386sDZhwj0v4Xi6uVEX7zvYv1DvEIvFn9hB2y6k2DHTaMz7k678
n8e/66iuYtIhwu7wlMdS0xBlw2GP9PDvg6PIaHGL9Zf2zrCrWFZzZTmdXdcOlci0
dLwdieYvWjItJkyRVIS4GNpmE9eBvt0uDEUTHGnmYLvefOuSKsdgcvCmYE1GAHd7
jH3GHSJ9SaZWq2E3dBeAmSqFS08QKiUZb/NJ78puUZ9w58oLPCeLhmqeDMnjINR4
adN+kJcNz8U3nk3MD2vsOPu/mnb3YPQ6vlsrUuAhPH+wQAjtdI7gQUsaUikBOqq/
342jF2UTiw5Po139KjShFI15/IUpZHa5td6GB2Umj/gqGq4qKucUpCEvdbJyOgpi
fCD2lif4rNpM06S9doLd58uF+k29Mq2Lo7qUrd0RtAMILnYdHCfSWFf/R2eRjtIR
R2/Xv9tm2C1gO9wnr1tFnvf8oMFuSOosMBm6yA+svwYoLlE4G7A2nDfyOmHT9H89
nBqjVTKJZXM684W6afoG7ndfKpsW0twNux62AUO615uggLbI04ITkZJOEuo0pdlY
Gvj2Z4Wt+ELJfOhbDON/B2qFdj9HuYRcblJZAVYfhczGtHtZSnwOmrhlOmHSAxux
Z7k+1chMW6iMrEeY4L3Z
=uF81
-----END PGP SIGNATURE-----
|