Обсуждение: pgbench is broken on strict-C89 compilers
I got around to trying to build PG with the HP-supplied compiler on my
ancient HPUX box, something I do about once a release cycle to see if
we've finally broken that trailing-edge toolchain. Things still seem
to work except for this:
cc: "pgbench.c", line 1579: error 1521: Incorrect initialization.
cc: "pgbench.c", line 1591: error 1521: Incorrect initialization.
What it's complaining about is these nonconstant initializers:
struct ddlinfo DDLs[] = { { "pgbench_history", scale >= SCALE_32BIT_THRESHOLD ?
"tidint,bid int,aid bigint,delta int,mtime timestamp,filler char(22)" : "tid int,bid int,aid int,delta
int,mtimetimestamp,filler char(22)", 0 },
which were apparently added in commit 89d00cbe. It appears to me that
the compiler is within its rights to refuse a nonconstant expression
for an inner initializer according to C89, though I don't see any such
restriction in C99.
We shipped this code in 9.3, and nobody's complained yet, so maybe
it's time to forget about C89 compliance. On the other hand, minor
notational convenience seems like a pretty poor reason to move the
goalposts for C language compliance, so I'm inclined to fix this.
I'm not entirely sure what the least ugly substitute code would be.
One idea is to replace the bigint/int column types with %s and fill
in the correct type with a snprintf operation, but that's not real
attractive because it would only work for a single such column,
and the compiler could not catch any format-spec-mismatch problems.
Thoughts?
regards, tom lane
Hi,
On 2014-05-17 19:15:15 -0400, Tom Lane wrote:
> I got around to trying to build PG with the HP-supplied compiler on my
> ancient HPUX box, something I do about once a release cycle to see if
> we've finally broken that trailing-edge toolchain. Things still seem
> to work except for this:
>
> cc: "pgbench.c", line 1579: error 1521: Incorrect initialization.
> cc: "pgbench.c", line 1591: error 1521: Incorrect initialization.
>
> What it's complaining about is these nonconstant initializers:
>
> struct ddlinfo DDLs[] = {
> {
> "pgbench_history",
> scale >= SCALE_32BIT_THRESHOLD
> ? "tid int,bid int,aid bigint,delta int,mtime timestamp,filler char(22)"
> : "tid int,bid int,aid int,delta int,mtime timestamp,filler char(22)",
> 0
> },
>
> which were apparently added in commit 89d00cbe. It appears to me that
> the compiler is within its rights to refuse a nonconstant expression
> for an inner initializer according to C89, though I don't see any such
> restriction in C99.
Yea, I've complained about it in
http://www.postgresql.org/message-id/20140403152834.GG17307@awork2.anarazel.de
That piece code is also confused about using static vs. const. For a lot
longer though...
I'd planned to put up a buildfarm animal compiling pg with
-Wc11-extensions and -Wc99-extensions once the configure bits are
committed in some form.
> We shipped this code in 9.3, and nobody's complained yet, so maybe
> it's time to forget about C89 compliance. On the other hand, minor
> notational convenience seems like a pretty poor reason to move the
> goalposts for C language compliance, so I'm inclined to fix this.
I think it's time to move past "pure" C89 in the near future. But I also
think if we do so we should do it at the beginning of the release
cycle. And selectively, using only the features we want and that are
widely available. E.g. msvc still doesn't fully support C99.
So +1 for fixing this.
> I'm not entirely sure what the least ugly substitute code would be.
> One idea is to replace the bigint/int column types with %s and fill
> in the correct type with a snprintf operation, but that's not real
> attractive because it would only work for a single such column,
> and the compiler could not catch any format-spec-mismatch problems.
I'd just duplicated the ddl structs. Seemed to be the least ugly thing I
could come up with. For from pretty tho.
Greetings,
Andres Freund
-- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services
Andres Freund <andres@2ndquadrant.com> writes: > On 2014-05-17 19:15:15 -0400, Tom Lane wrote: >> ... It appears to me that >> the compiler is within its rights to refuse a nonconstant expression >> for an inner initializer according to C89, though I don't see any such >> restriction in C99. > Yea, I've complained about it in > http://www.postgresql.org/message-id/20140403152834.GG17307@awork2.anarazel.de Ah. I'd sort of ignored that patch because it didn't seem too relevant to the issues we were discussing at the time. > That piece code is also confused about using static vs. const. For a lot > longer though... Well, "static" is also a good thing here because it eliminates the need for runtime initialization of a function-local array variable. But yeah, the code is way under-const-ified as well. > I'd just duplicated the ddl structs. Seemed to be the least ugly thing I > could come up with. For from pretty tho. It works, anyway. If I don't think of something better, I'll do a bit more polishing and commit that tomorrow. regards, tom lane
On 2014-05-18 00:36:34 -0400, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > On 2014-05-17 19:15:15 -0400, Tom Lane wrote: > >> ... It appears to me that > >> the compiler is within its rights to refuse a nonconstant expression > >> for an inner initializer according to C89, though I don't see any such > >> restriction in C99. > > > Yea, I've complained about it in > > http://www.postgresql.org/message-id/20140403152834.GG17307@awork2.anarazel.de > > Ah. I'd sort of ignored that patch because it didn't seem too relevant > to the issues we were discussing at the time. Well, I wanted to start a animal that quickly warns about the nameless union stuff. That'd require a sensibly clean build. > > That piece code is also confused about using static vs. const. For a lot > > longer though... > > Well, "static" is also a good thing here because it eliminates the need > for runtime initialization of a function-local array variable. But yeah, > the code is way under-const-ified as well. Well, const alone would do that too; without requiring checks on every function invocation. The const variant should end up in the readonly part of the binary... > > I'd just duplicated the ddl structs. Seemed to be the least ugly thing I > > could come up with. For from pretty tho. > > It works, anyway. If I don't think of something better, I'll do a bit > more polishing and commit that tomorrow. Ok, thanks. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services