pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Michael Paquier <michael@paquier.xyz>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Thomas Munro <thomas.munro@enterprisedb.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Allowing printf("%m") only where it actually works
Date: Fri, 14 Sep 2018 11:02:56 +0900
Message-ID: <20180914020256.GD29332@paquier.xyz> (raw)
In-Reply-To: <1304.1536774001@sss.pgh.pa.us>
References: <20180528150125.j2x3mp62ssbeg3h5@alvherre.pgsql>
	<2685.1527520461@sss.pgh.pa.us>
	<8770.1533931511@sss.pgh.pa.us>
	<16896.1533935042@sss.pgh.pa.us>
	<14925.1534040609@sss.pgh.pa.us>
	<1437.1534100930@sss.pgh.pa.us>
	<13183.1534624490@sss.pgh.pa.us>
	<30022.1534705920@sss.pgh.pa.us>
	<20180912070739.GK25160@paquier.xyz>
	<1304.1536774001@sss.pgh.pa.us>

On Wed, Sep 12, 2018 at 01:40:01PM -0400, Tom Lane wrote:
> Michael Paquier <michael@paquier.xyz> writes:
> > I would have liked to look at this patch in details, but it failed to
> > apply.  Could you rebase?
> 
> Ah, yeah, the dlopen patch touched a couple of the same places.
> Rebase attached --- no substantive changes.

-       if (handleDLL == NULL)
-           ereport(FATAL,
-                   (errmsg_internal("could not load netmsg.dll: error
-            code %lu", GetLastError())));

In 0001, this is replaced by a non-FATAL error for the backend, which
does not seem like a good idea to me because the user loses visibility
with this DDL which canot be loaded.  I still have to see this error...

And about 0002.  I am surprised by the amount of cleanup that the
removal of all the special wrappers for %m gives, especially
expand_fmt_string.  So +1 for the concept.  I have been testing both
patches individually on Windows and compilation, as well as tests, do
not show any issues.  The tests have been done only with MSVC.

Could you drop the configure checks for snprintf and vsnprintf in a
separate patch?  The cleanup of %m and the removal of those switches
should be treated separatly in my opinion.
--
Michael

Attachments:

  signature.asc (application/pgp-signature, 832B) download

view thread (50+ messages)  latest in thread

Message-ID: <20180914020256.GD29332@paquier.xyz>
Permalink:  ../20180914020256.GD29332@paquier.xyz/
Also on:    postgresql.org/message-id/20180914020256.GD29332@paquier.xyz

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: michael@paquier.xyz, tgl@sss.pgh.pa.us, thomas.munro@enterprisedb.com, alvherre@2ndquadrant.com, pgsql-hackers@lists.postgresql.org
  Subject: Re: Allowing printf("%m") only where it actually works
  In-Reply-To: <20180914020256.GD29332@paquier.xyz>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox