[PATCH v3 0/1] MR10199: msvcrt: Fix no buffering detection in _fwrite_nolock().
-- v3: msvcrt: Fix no buffering detection in _fwrite_nolock(). https://gitlab.winehq.org/wine/wine/-/merge_requests/10199
From: Paul Gofman <pgofman@codeweavers.com> --- dlls/msvcrt/file.c | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/dlls/msvcrt/file.c b/dlls/msvcrt/file.c index 01853625c23..22301d58728 100644 --- a/dlls/msvcrt/file.c +++ b/dlls/msvcrt/file.c @@ -4147,10 +4147,12 @@ size_t CDECL fwrite(const void *ptr, size_t size, size_t nmemb, FILE* file) size_t CDECL _fwrite_nolock(const void *ptr, size_t size, size_t nmemb, FILE* file) { size_t wrcnt=size * nmemb; - int written = 0; + int written = 0, bufsize; if (size == 0) return 0; + bufsize = !(file->_flag & (MSVCRT__NOBUF | _IOMYBUF | MSVCRT__USERBUF)) && !msvcrt_alloc_buffer(file) ? 1 : file->_bufsiz; + while(wrcnt) { if(file->_cnt < 0) { WARN("negative file->_cnt value in %p\n", file); @@ -4164,20 +4166,9 @@ size_t CDECL _fwrite_nolock(const void *ptr, size_t size, size_t nmemb, FILE* fi written += pcnt; wrcnt -= pcnt; ptr = (const char*)ptr + pcnt; - } else if((file->_flag & MSVCRT__NOBUF) - || ((file->_flag & (_IOMYBUF | MSVCRT__USERBUF)) && wrcnt >= file->_bufsiz) - || (!(file->_flag & (_IOMYBUF | MSVCRT__USERBUF)) && wrcnt >= MSVCRT_INTERNAL_BUFSIZ)) { + } else if(wrcnt >= bufsize) { size_t pcnt; - int bufsiz; - - if(file->_flag & MSVCRT__NOBUF) - bufsiz = 1; - else if(!(file->_flag & (_IOMYBUF | MSVCRT__USERBUF))) - bufsiz = MSVCRT_INTERNAL_BUFSIZ; - else - bufsiz = file->_bufsiz; - - pcnt = (wrcnt / bufsiz) * bufsiz; + pcnt = (wrcnt / bufsize) * bufsize; if(msvcrt_flush_buffer(file) == EOF) break; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10199
v3: - use Piotr's expression (also treat _NOBUF as buffered write). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10199#note_130737
Piotr Caban (@piotr) commented about dlls/msvcrt/file.c:
size_t CDECL _fwrite_nolock(const void *ptr, size_t size, size_t nmemb, FILE* file) { size_t wrcnt=size * nmemb; - int written = 0; + int written = 0, bufsize; if (size == 0) return 0;
+ bufsize = !(file->_flag & (MSVCRT__NOBUF | _IOMYBUF | MSVCRT__USERBUF)) && !msvcrt_alloc_buffer(file) ? 1 : file->_bufsiz; It's currently the longest line in the file - could you please split it?
Thinking about it more, maybe following code is easier to understand: ```c if(!(file->_flag & (MSVCRT__NOBUF | _IOMYBUF | MSVCRT__USERBUF))) msvcrt_alloc_buffer(file); bufsize = file->_bufsiz > 0 ? file->_bufsiz : 1; ``` What do you think? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10199#note_130741
On Fri Feb 27 16:47:13 2026 +0000, Piotr Caban wrote:
It's currently the longest line in the file - could you please split it? Thinking about it more, maybe following code is easier to understand: ```c if(!(file->_flag & (MSVCRT__NOBUF | _IOMYBUF | MSVCRT__USERBUF))) msvcrt_alloc_buffer(file); bufsize = file->_bufsiz > 0 ? file->_bufsiz : 1; ``` What do you think? This changes the logic, in the absence of alloc_buffer call it relies on file->bufsiz ignoring flags. That’s probably fine but given the interest of some apps in fiddling with file structs directly I personally don’t see such a difference as no-op. Maybe I will just split ternary expression with ‘if’ (I also think too complicated ternary expressions are less readable) but keep flag checks?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10199#note_130743
On Fri Feb 27 16:54:12 2026 +0000, Paul Gofman wrote:
This changes the logic, in the absence of alloc_buffer call it relies on file->bufsiz ignoring flags. That’s probably fine but given the interest of some apps in fiddling with file structs directly I personally don’t see such a difference as no-op. Maybe I will just split ternary expression with ‘if’ (I also think too complicated ternary expressions are less readable) but keep flag checks? Like this:
int written = 0, bufsize = 1;
if (size == 0)
return 0;
if ((file->_flag & (MSVCRT__NOBUF | _IOMYBUF | MSVCRT__USERBUF)) || msvcrt_alloc_buffer(file))
bufsize = file->_bufsiz;
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10199#note_130745
On Fri Feb 27 16:58:50 2026 +0000, Paul Gofman wrote:
Like this: ``` int written = 0, bufsize = 1; if (size == 0) return 0; if ((file->_flag & (MSVCRT__NOBUF | _IOMYBUF | MSVCRT__USERBUF)) || msvcrt_alloc_buffer(file)) bufsize = file->_bufsiz; ``` Yes, in theory it can affect applications that set file-\>\_bufsiz themself (but only for stdout or stderr case, otherwise msvcrt_alloc_buffer would have overwritten it). Taking that into account I think the risk is small. Anyway, above code looks good for me.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10199#note_130746
participants (3)
-
Paul Gofman -
Paul Gofman (＠gofman)
-
Piotr Caban (＠piotr)