On Sunday 21 June 2026 09:57:27 Kirill Makurin wrote:
> From: Pali Rohár <[email protected]>
> 
> > On Sunday 21 June 2026 06:25:22 Kirill Makurin wrote:
> >> Pali Rohár <[email protected]> wrote:
> >> > Currently it first tries to estimate buffer size based on wcsnlen and
> >> > MB_LEN_MAX. Then it calculates the real buffer size correctly via
> >> > WideCharToMultiByte() and third time it is using WideCharToMultiByte()
> >> > for the final conversion. So the first step via wcsnlen and MB_LEN_MAX
> >> > is not needed, the real size can be calculated directly.
> >>
> >> I think you misunderstood the purpose of `wcsnlen` trick.
> >>
> >> So, basically, we limit `_alloca` allocation size to `BUFSIZ` for 
> >> `_Message` arg and to `FILENAME_MAX` for `_File` arg. The `wcsnlen` trick 
> >> is used to allocate smaller buffer when we can ensure that length of 
> >> converted string will never exceed `BUFSIZ` or `FILENAME_MAX`. It is an 
> >> attempt to reduce possibility of stack overflow. While doing so, I 
> >> overlooked a stupid off-by-one error...
> >
> >I understood it. The first call to WideCharToMultiByte() just calculate
> > the total length of buffer required to do full conversion. It is doing
> > without any side-effect and without writing anything to do buffer.
> > So we can use lenght := min(output_from_WideCharToMultiByte, BUFSIZ) and
> > then use this length for passing to alloca (with +1 or something like
> > that). And with this we allocate smaller buffer when not full BUFSIZ is
> > needed and also at the same time we do not need to call wcsnlen and do
> > some estimation.
> 
> The issue with `min(output_from_WideCharToMultiByte, BUFSIZ)` is that if we 
> end up passing BUFSIZ to `WideCharToMultibyte`, it may end up writing less 
> bytes that `BUFSIZ`; for example, if it cannot fit a multibyte character into 
> buffer. Those few bytes in the end of the buffer will have random values from 
> the stack.

The WideCharToMultiByte is returning length which is number of
filled bytes in output buffer. We should use this return value to
properly fill the nul term after the last filled byte and then it should
be safe. Moreover it always a good idea to fill that nul term after the
WideCharToMultiByte call as it does not have to fill it automatically.

So the idea is:

1. Call WideCharToMultiByte to just calculate length of buffer required
   to convert all characters.

2. Reduce the calculated length of buffer to BUFSIZ if it is larger.

3. Allocate buffer on stack of length from step 2.

4. Call WideCharToMultiByte to do actual conversion with the output
   buffer length from step 2 to the buffer from step 3. It will return
   number of written bytes.

5. Fill the nul term after the last written byte determined from step 4.
   (In case the last written byte was at the last byte of buffer then
    write hat nul term at the last byte of buffer.)

I quite do not see where is problem with usage of uninitialized (random)
values from stack/buffer. Even if it writes less bytes into output
buffer we still properly fill the nul term.

> >> > Also memset is not needed.
> >>
> >> I'm pretty sure it is needed. `WideCharToMultiByte` writes NUL to output 
> >> buffer only if it was converted.
> >>
> >> Consider string "テクスト" whose length is 12 code units (+NUL) in UTF-8 or 8 
> >> code units (+NUL) in code page 932. Now, if you call
> >>
> >>     WideCharToMultiByte (CP_UTF8, 0, L"テクスト", -1, buf, 10, NULL, NULL)
> >>
> >> It will write "テクス" to `buf` and not NUL-terminate it. Call to `memset` 
> >> ensures that string will always be NUL-terminated.
> >
> > I see. In my proposed change I always put the explicit nul term after
> > the WideCharToMultiByte() call at the end of conversion, to be sure that
> > it is always nul term. So only with this change it is not needed.
> 
> - Kirill Makurin


_______________________________________________
Mingw-w64-public mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public

Reply via email to