Re: [3/3] msvcrt: Implement 4k block flushing behavior.
On 05/31/14 01:00, Grazvydas Ignotas wrote:
+ } else if(wrcnt >= MSVCRT_WRITE_BLOCK_SIZE) { + if(msvcrt_set_write_direction(file) != 0) + break; + + if(file->_bufsiz && file->_ptr != file->_base) + if(msvcrt_flush_buffer(file) == MSVCRT_EOF) + break; + + if(MSVCRT__write(file->_file, ptr, MSVCRT_WRITE_BLOCK_SIZE) <= 0) { + file->_flag |= MSVCRT__IOERR; + break; + } + written += MSVCRT_WRITE_BLOCK_SIZE; + wrcnt -= MSVCRT_WRITE_BLOCK_SIZE; + ptr = (const char*)ptr + MSVCRT_WRITE_BLOCK_SIZE; This doesn't look right for me. It doesn't make sense to have some special size blocks written in different way.
I think this is caused by 2 differences in native and wine's implementation of fwrite: - if you check file->_bufsiz value on windows it's set to 4096 (on wine it's 512), this is probably causing the flash to be done when 4096 bytes are written - probably there's some kind of optimization that writes integral number of buffers directly to file - I was not checking it but maybe native flushes the output when buffer is full, wine will exit fwrite with full buffer in this case I've also seen that _bufsiz is set to 0 on wine if buffer was not yet allocated. On windows it's value is 4096 just after opening the file. Cheers, Piotr
On Sun, Jun 1, 2014 at 12:38 PM, Piotr Caban <piotr.caban(a)gmail.com> wrote:
On 05/31/14 01:00, Grazvydas Ignotas wrote:
+ } else if(wrcnt >= MSVCRT_WRITE_BLOCK_SIZE) { + if(msvcrt_set_write_direction(file) != 0) + break; + + if(file->_bufsiz && file->_ptr != file->_base) + if(msvcrt_flush_buffer(file) == MSVCRT_EOF) + break; + + if(MSVCRT__write(file->_file, ptr, MSVCRT_WRITE_BLOCK_SIZE) <= 0) { + file->_flag |= MSVCRT__IOERR; + break; + } + written += MSVCRT_WRITE_BLOCK_SIZE; + wrcnt -= MSVCRT_WRITE_BLOCK_SIZE; + ptr = (const char*)ptr + MSVCRT_WRITE_BLOCK_SIZE;
This doesn't look right for me. It doesn't make sense to have some special size blocks written in different way.
I think this is caused by 2 differences in native and wine's implementation of fwrite: - if you check file->_bufsiz value on windows it's set to 4096 (on wine it's 512), this is probably causing the flash to be done when 4096 bytes are written
I don't think so, as doing 2K fwrites 2 times (for example) doesn't trigger the flush, only a single write >= 4K does.
- probably there's some kind of optimization that writes integral number of buffers directly to file
Yes and this is what I'm trying to implement/simulate here..
- I was not checking it but maybe native flushes the output when buffer is full, wine will exit fwrite with full buffer in this case
As mentioned above, no it does not. Doing it this way would fix IDA, but I fear it may break something else. IDA does 8K writes at random offsets of it's database file, then does fileno() and some lseeks, after that it goes back to using stdio (does fseek() first), but at that point the flush will go to last lseek it did and corrupt it's database. I guess defaulting to 4K buffer and flushing it if it's full on fwrite() exit would be closer to how msvcrt operates, even though not entirely correct. Do you prefer this solution, or perhaps something entirely different? -- Gražvydas
On 06/03/14 01:16, Grazvydas Ignotas wrote:
I guess defaulting to 4K buffer and flushing it if it's full on fwrite() exit would be closer to how msvcrt operates, even though not entirely correct. Do you prefer this solution, or perhaps something entirely different? I would like to have it implemented as similar as possible to native dll. So taking in account that two 2k block writes doesn't flush output I don't want to do it in wine.
I've added following call to your test (just after the file is opened): setvbuf(file, NULL, _IOFBF, 512); It causes following test failures on windows 7: file.c:2250: Test failed: unexpected flush by 512 byte write file.c:2250: Test failed: unexpected flush by 1024 byte write file.c:2250: Test failed: unexpected flush by 1536 byte write ... This shows that your code should depend on buffer size instead of MSVCRT_WRITE_BLOCK_SIZE. Note: keep in mind that wine's implementation of setvbuf is quite broken, it's not allocating new buffer if second argument is NULL. Thanks, Piotr
On Tue, Jun 3, 2014 at 11:56 AM, Piotr Caban <piotr.caban(a)gmail.com> wrote:
On 06/03/14 01:16, Grazvydas Ignotas wrote:
I guess defaulting to 4K buffer and flushing it if it's full on fwrite() exit would be closer to how msvcrt operates, even though not entirely correct. Do you prefer this solution, or perhaps something entirely different?
I would like to have it implemented as similar as possible to native dll. So taking in account that two 2k block writes doesn't flush output I don't want to do it in wine.
I've added following call to your test (just after the file is opened): setvbuf(file, NULL, _IOFBF, 512);
It causes following test failures on windows 7: file.c:2250: Test failed: unexpected flush by 512 byte write file.c:2250: Test failed: unexpected flush by 1024 byte write file.c:2250: Test failed: unexpected flush by 1536 byte write ... This shows that your code should depend on buffer size instead of MSVCRT_WRITE_BLOCK_SIZE.
Ok then, I'll be sending next revision with the same approach but make it depend on buffer size.
Note: keep in mind that wine's implementation of setvbuf is quite broken, it's not allocating new buffer if second argument is NULL.
Sounds like i can take a stab at that too while at it. -- Gražvydas
participants (2)
-
Grazvydas Ignotas -
Piotr Caban