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

* Michael Paquier wrote:

> On Thu, Sep 1, 2016 at 4:03 PM, Christian Ullrich <chris(at)chrullrich(dot)net> wrote:

>> 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.

Well, I still maintain that if it doesn't work and has never worked,
getting rid of it is better than making it work six years after the
fact. OTOH, there may have been cases where it did actually work,
perhaps those gnuwin32 libraries that were mentioned in the comment
before it was changed in that commit above, if they were loaded before
the first call to the function.

OTTH, wouldn't it be funny if fixing it actually broke something that
worked accidentally because it *didn't* get the updated environment? I
think that is at least as likely as suddenly getting excited reports
that something now works that hasn't before.

> It is better to avoid !!. See for example 1a7a436 that avoided
> problems with VS2015 as far as I recall.

Agreed, thanks for noticing. This change creates a warning, however,
because GetModuleHandleEx() returns BOOL, not HMODULE. Updated 0003
attached, simplified over my original one.

> 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?

In lieu of convincing you to drop the entire thing, yes, that looks
about right, except for the BOOL thing.

--
Christian