[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