Re: pgsql: Add putenv support for msvcrt from Visual Studio 2013

On Thu, Sep 1, 2016 at 4:03 PM, Christian Ullrich <chris@chrullrich.net> wrote:
> * Michael Paquier wrote:
>
>> On Wed, Apr 27, 2016 at 2:39 AM, Christian Ullrich <chris@chrullrich.net>
>> wrote:
>
>>> * Christian Ullrich wrote:
>
>> And actually, by looking at those patches, isn't it a dangerous
>> practice to be able to load multiple versions of the same DLL routines
>> in the same workspace? I have personally very bad souvenirs with that,
>
> No, it is exactly what the version-specific CRTs are meant to allow. Each
> module uses the CRT version it needs, and they don't share any state, so
> absent bugs, they cannot conflict.

Hm. OK.

> That said, introducing this requirement would be a very significant change.
> I'm not sure how many independently maintained compiled extensions there
> are, but this would mean that their developers would have to have the
> matching VS versions for every PG distribution they want to support. Even if
> that's just EDB, it still is a lot of effort.

Yes. FWIW in my stuff everything gets compiled based on the same VS
version and bundled in the same msi, including a set of extensions
compiled from source, but perhaps my sight is too narrow in this
area... Well let's forget about that.

> My conclusion from April stands:
>
>> The fact that master looks like it does means that there have not been
>> many (or any) complaints about missing cross-module environment
>> variables. If nobody ever needs to see a variable set elsewhere, we
>> have a very simple solution: Why don't we simply throw out the whole
>> #ifdef _MSC_VER block?

Looking at the commit logs and 741e4ad7 that does not sound like a good idea.

+                   if (!rtmodules[i].pinned)
+                   {
+                       HMODULE tmp;
+                       BOOL res = GetModuleHandleEx(
+                               GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS
+                                   | GET_MODULE_HANDLE_EX_FLAG_PIN,
+                               (LPCTSTR)rtmodules[i].hmodule,
+                               &tmp);
+                       rtmodules[i].pinned = !!res;
+                   }
It is better to avoid !!. See for example 1a7a436 that avoided
problems with VS2015 as far as I recall.

In order to avoid any problems with the load and unload windows, my
bet goes for 0001, 0002 and 0003, with the last two patches merged
together, 0001 being only a set of independent fixes. That's ugly, but
that looks the safest course of actions. I have rebased/rewritten the
patches as attached. Thoughts?
--
Michael