From: "eric pouech (@epo)" <wine@gitlab.winehq.org>
Subject: Re: [PATCH 0/1] MR352: cmd: Don't loop infinitely on NUL.
Message-Id: <wine-wine-mr352-v1-note3109@gitlab-mail-bridge>
Date: Fri, 01 Jul 2022 10:00:54 +0000
In-Reply-To: <wine-wine-mr352-v1@gitlab-mail-bridge>
References: <merge_request_431@gitlab.winehq.org> <wine-wine-mr352-v1@gitlab-mail-bridge>

Hi Arek,

After testing a bit more locally, what I get is:
- no support for ctrl-z in for /F handling
- current code (before your patch) is looping endlessly in WCMD_fgets when a stray NUL character is read from a file
- native cmd.exe behavior (tested in W11) is inconsistent
  + native cmd stops reading a line with a stray NUL (like your tests show) and throws it away
  + native cmd loops endlessly in some cases (like when parsing a .bat/.cmd file) ("echo\necho\0echo" thingie)

So it boils down to what error handling we put in place in case of a NUL character in text file.

Your patch implies:
- never loop endlessly for all callers on NUL character
- NUL is an EOF marker
- throw away previous characters on line (if any)

So I'd rather fix WCMD_fgets with attached [patch](/uploads/24ba4006fb80e1460e6f0251e0dd9160/err):
- never loop endlessly for all callers on NUL character (not consistent with native, but we can live with that)
- NUL is not an EOF marker
- just include the NUL in the returned buffer
  + so if caller cares (like in 'for /F' handler), he can take appropriate actions
  + if caller doesn't care (like the other actual callers in builtin cmd.exe, he will get the characters before the NUL, and continue on next line

But I agree that we likely haven't been hit by the differences above as it would be still be looping endlessly <g>.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/352#note_3109