On Sep 9 22:40, Takashi Yano via Cygwin-patches wrote:
> - Use tmp_pathbuf instead of HeapAlloc()/HeapFree().
> - Remove mb_str_free() function.
> - Consider the case where the multibyte string stops in the middle
> of a multibyte char.
> ---
> winsup/cygwin/fhandler_tty.cc | 118 ++++++++++++++++++++++------------
> 1 file changed, 77 insertions(+), 41 deletions(-)
>
> diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
> index 6de591d9b..c4b7fc61d 100644
> --- a/winsup/cygwin/fhandler_tty.cc
> +++ b/winsup/cygwin/fhandler_tty.cc
> @@ -26,6 +26,7 @@ details. */
> #include <asm/socket.h>
> #include "cygwait.h"
> #include "registry.h"
> +#include "tls_pbuf.h"
>
> #ifndef PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE
> #define PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE 0x00020016
> @@ -116,40 +117,72 @@ CreateProcessW_Hooked
> return CreateProcessW_Orig (n, c, pa, ta, inh, f, e, d, si, pi);
> }
>
> -static char *
> -convert_mb_str (UINT cp_to, size_t *len_to,
> - UINT cp_from, const char *ptr_from, size_t len_from)
> +static void
> +convert_mb_str (UINT cp_to, char *ptr_to, size_t *len_to,
> + UINT cp_from, const char *ptr_from, size_t len_from,
> + mbstate_t *stat)
Heh, it's funny to use mbstate_t together with Windows functions :)
However, the var name `stat' makes me itch. It looks too close to
struct stat and function stat. What about "mbp" or something along
these lines?
> {
> - char *buf;
> size_t nlen;
> + tmp_pathbuf tp;
> if (cp_to != cp_from)
> {
> - size_t wlen =
> - MultiByteToWideChar (cp_from, 0, ptr_from, len_from, NULL, 0);
> - wchar_t *wbuf = (wchar_t *)
> - HeapAlloc (GetProcessHeap (), 0, wlen * sizeof (wchar_t));
> - wlen =
> - MultiByteToWideChar (cp_from, 0, ptr_from, len_from, wbuf, wlen);
> - nlen = WideCharToMultiByte (cp_to, 0, wbuf, wlen, NULL, 0, NULL, NULL);
> - buf = (char *) HeapAlloc (GetProcessHeap (), 0, nlen);
> - nlen = WideCharToMultiByte (cp_to, 0, wbuf, wlen, buf, nlen, NULL,
> NULL);
> - HeapFree (GetProcessHeap (), 0, wbuf);
> + wchar_t *wbuf = tp.w_get ();
> + int wlen = 0;
> + if (cp_from == 65000) /* UTF-7 */
CP_UTF7?
> + /* MB_ERR_INVALID_CHARS does not work properly for UTF-7.
> + Therefore, just convert string without checking */
> + wlen = MultiByteToWideChar (cp_from, 0, ptr_from, len_from,
> + wbuf, NT_MAX_PATH);
> + else
> + {
> + char *tmpbuf = tp.c_get ();
> + memcpy (tmpbuf, stat->__value.__wchb, stat->__count);
> + if (stat->__count + len_from > NT_MAX_PATH)
> + len_from = NT_MAX_PATH - stat->__count;
> + memcpy (tmpbuf + stat->__count, ptr_from, len_from);
> + int total_len = stat->__count + len_from;
> + stat->__count = 0;
> + int mblen = 0;
> + for (const char *p = tmpbuf; p < tmpbuf + total_len; p += mblen)
> + /* Max bytes in multibyte char is 4. */
> + for (mblen = 1; mblen <= 4; mblen ++)
> + {
> + /* Try conversion */
> + int l = MultiByteToWideChar (cp_from, MB_ERR_INVALID_CHARS,
> + p, mblen,
> + wbuf + wlen, NT_MAX_PATH - wlen);
> + if (l)
> + { /* Conversion Success */
> + wlen += l;
> + break;
> + }
> + else if (mblen == 4)
> + { /* Conversion Fail */
> + l = MultiByteToWideChar (cp_from, 0, p, 1,
> + wbuf + wlen, NT_MAX_PATH - wlen);
> + wlen += l;
> + mblen = 1;
> + break;
> + }
> + else if (p + mblen == tmpbuf + total_len)
> + { /* Multibyte char incomplete */
> + memcpy (stat->__value.__wchb, p, mblen);
> + stat->__count = mblen;
> + break;
> + }
> + /* Retry conversion with extended length */
> + }
> + }
> + nlen = WideCharToMultiByte (cp_to, 0, wbuf, wlen,
> + ptr_to, *len_to, NULL, NULL);
> }
> else
> {
> /* Just copy */
> - buf = (char *) HeapAlloc (GetProcessHeap (), 0, len_from);
> - memcpy (buf, ptr_from, len_from);
> - nlen = len_from;
> + nlen = min (*len_to, len_from);
> + memcpy (ptr_to, ptr_from, nlen);
if the *caller* checks for cp_to != cp_from, this memcpy could go
away and the caller could just use the already existing buffer.
Other than that, LGTM.
Thanks,
Corinna
