Horton

Re: BUG #15080: ecpg on windows doesn't define HAVE_LONG_LONG_INT

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Huong Dangminh <huo-dangminh@ys.jp.nec.com>
Cc: Andrew Dunstan <andrew@dunslane.net>, Jonathan Allen <jallen@americansavingslife.com>, Andrew Gierth <andrew@tao11.riddles.org.uk>, "pgsql-bugs@lists.postgresql.org" <pgsql-bugs@lists.postgresql.org>, Michael Meskes <meskes@postgresql.org>, Akio Iwaasa <aki-iwaasa@vt.jp.nec.com>
Date: 2018-05-19T16:45:00Z
Lists: pgsql-bugs 
Huong Dangminh <huo-dangminh@ys.jp.nec.com> writes:
>> From: Tom Lane [mailto:tgl@sss.pgh.pa.us]
>> It seems like really we need <ecpg_config.h> in sqltypes.h at least, and
>> if we don't want more bugs of the same ilk in future, it'd be wise to stick
>> it into something that's included by all ecpg-generated code, like ecpglib.h.
>> I am however worried about invasion of client namespace if we do that, so
>> not quite sure what to do here.  Thoughts?

> I am also inclined to the above, by the way.

Any other opinions about that?  These are the symbols that ecpg_config.h
might define, at present:

HAVE_INT64
HAVE_LONG_INT_64
HAVE_LONG_LONG_INT
HAVE_LONG_LONG_INT_64
ENABLE_THREAD_SAFETY

This doesn't seem like a huge invasion of client namespace, but it's
not entirely zero risk either.

One idea is to leave the back branches as they now stand, but fix this
issue in HEAD.  That would leave the hazard in sqltypes.h unfixed in
the back branches ... but given the lack of field complaints, a fix there
could be worse than the disease.

> Also thanks for fixing the failed of "Buildfarm member dromedary".
> # I also feel curious about it.

We still have two issues in the buildfarm:

* gaur/pademelon fell over last night for lack of strtoll().  My first
response to that was to disable the ecpg tests on those critters,
reasoning that it couldn't possibly be worth rolling our own strtoll()
in 2018.  However, further digging shows that the functionality *is*
available, it's just spelled __strtoll.  So now I'm inclined to teach
configure about that and undo the lobotomization of those animals.
If we're still supporting strtoq() we can surely manage __strtoll().

* frogmouth is failing the test too, due to printing values that look
like the expected 64-bit result has been truncated to 32 bits.
I suspect this explains it:

  sqlda.pgc: In function 'dump_sqlda':
  sqlda.pgc:44:4: warning: unknown conversion type character 'l' in format
  sqlda.pgc:44:4: warning: too many arguments for format

I'm guessing that the native printf() on that platform doesn't know
"%lld" and is printing the value as 32 bits.  This seems a bit
problematic to fix.  Using INT64_FORMAT isn't a solution: it's a cheat
in that it's assuming that "long long" is 64 bits, and what's more,
it definitely won't fix the problem on frogmouth because that build is
using our own snprintf and so its value of INT64_FORMAT is calibrated
to work with our code, ie, it's going to be "%lld" anyway.

However ... we've got a ton of other places that use INT64_FORMAT with
the native printf, eg in pgbench, and frogmouth is not producing
warnings about those usages.  So I'm confused about exactly what is
happening there.  Andrew, do you have any insight?

			regards, tom lane

Commits

  1. Remove configure's check for nonstandard "long long" printf modifiers.

  2. printf("%lf") is not portable, so omit the "l".

  3. Support platforms where strtoll/strtoull are spelled __strtoll/__strtoull.

  4. Arrange to supply declarations for strtoll/strtoull if needed.

  5. Hot-fix ecpg regression test for missing ecpg_config.h inclusion.

  6. Add some test coverage for ecpg's "long long" support.

  7. Recognize that MSVC can support strtoll() and strtoull().

  8. Fix up ecpg's configuration so it handles "long long int" in MSVC builds.