Re: [ros-dev] Re: [ros-diffs] [weiden] 16536: return the length of the string excluding the null-termination character on success in GetEnvironmentVariable(). Thanks to Hartmut.

[Thomas Weidenmueller]

Joseph Galbraith wrote:
>   If I read the code in RtlQueryEnvironmentVariable_U
>  correctly (and I don't swear I have), it is possible
>  for it to return STATUS_SUCCESS and not copy a
>  NUL terminator.  This happens if the buffer is large
>  enough to contain the value but not the NUL terminator. 
That is true and needs to be fixed.

>   But then, if I read the code in
> GetEnvironmentVariable()
>  correctly, I don't believe it takes this situation into
>  account. 
Agreed, see above.

>   Probably, it would be best if
> RtlQueryEnvironmentVariable_U
>  _never_ copied the NUL terminator, and the caller was
>  always responsible for adding NUL termination, if needed. 
No, it has to but it depends on whether there's enough space.

My proposal to fix the issue is that we change:

    VarValue.MaximumLength = nSize * sizeof(WCHAR);

in GetEnvironmentVariableW to:

    VarValue.MaximumLength = (nSize - 1) * sizeof(WCHAR);

Of course, similar changes would have to be applied to the ansi version.
However, the case where nSize == 0 (and maybe also 1) would have to be
handled appropriately. This way (given we set the termination character
after the RtlQueryEnvironmentVariable_U call) we'd ensure the string is
always NULL-terminated because RtlQueryEnvironmentVariable_U only
terminates it if MaximumLength > Length.

Best Regards,
Thomas