Re: [PATCH v3 0/4] MR4313: MSVCRT _(w)environ[] has a separate allocation chunk for each entry.

[Piotr Caban (＠piotr)]

Piotr Caban (@piotr) commented about dlls/msvcrt/data.c:
> - {
> -
>    if (!memcmp( name, MSVCRT__environ[i], len ) && MSVCRT__environ[i][len] == '=')
> -
>    {
> -
>        free( MSVCRT__environ[i] );
> -
>        if (MSVCRT__wenviron) free( MSVCRT__wenviron[i] );
> -
>        if (set)
> -
>        {
> -
>            MSVCRT__environ[i] = set;
> -
>            if (MSVCRT__wenviron) MSVCRT__wenviron[i] = msvcrt_wstrdupa( set );
> -
>        }
> -
>        else
> -
>        {
> -
>            do
> -
>            {
> -
>                MSVCRT__environ[i] = MSVCRT__environ[i + 1];
> -
>                if (MSVCRT__wenviron) MSVCRT__wenviron[i] = MSVCRT__wenviron[i + 1];

This code is making assumption that _environ and _wenviron are always in sync, I think it's OK but we should make sure that's true. We would at least need to make sure that msvcrt_wstrdupa failure is handled correctly. I didn't dig into it but if msvcrt_wstrdupa fails it can cause some environment variables being removed. It can also cause some leaks.