[cmake-developers] [PATCH] Improve encoding handling on Windows

[Dāvis Mosāns]

2016-07-01 15:25 GMT+03:00 Daniel Pfeifer <daniel at pfeifer-mail.de>:

> Hi Dāvis,
>
> On Fri, Jul 1, 2016 at 4:18 AM, Dāvis Mosāns <davispuh at gmail.com> wrote:
> > On Windows getenv uses ANSI codepage so it needs to be encoded to
> > internally used encoding (eg. UTF-8). Here we use _wgetenv instead
> > and encode that.
>
> Your change to the SystemTools::GetEnv function introduces memory
> leaks, since you return a pointer to a new[]-ed array.
> It seems impossible to refactor SystemTools::GetEnv to use _wgetenv
> and provide interface compatability.
> You should probably introduce a function that returns std::string and
> uses GetEnvironmentVariableW internally.
>
>
I know about memory leak, I intentionally left it this way for first version
of patch as there are multiple ways how to implement it properly.

POSIX's getenv doesn't need to be freed, but here for Windows since we need
to encode it to internal encoding we'll be returning a new pointer.

I see two ways:

1. add SystemTools::FreeEnv which delete's if on Windows but does nothing
  otherwise;
2. change GetEnv to return std::unique_ptr<std:string> which will be
  automatically deleted once out of scope and we still can check if there
  wasn't such env if it's empty.

To me 2nd option seems best.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/cmake-developers/attachments/20160701/f0be1d32/attachment.html>