Warning is adjusted of pgbench.

On Tue, Aug 07, 2007 at 04:58:19PM +0900, Hiroshi Saito wrote:

Hi Magnus.

pgbench.c: In function `main':
pgbench.c:1257: warning: implicit declaration of function `getopt'

adjustment of some reference is required for this.
and this is a FRONTEND program.
patch is smooth at VC8 and MinGW (gcc).

Hi!

Why do you need to #undef EXEC_BACKEND, and is there a specific reason for
removing the include of win32.h?

//Magnus

Hi. Magnus

$ make
gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Winline -Wdeclaration-after-statement -Wendif-labels
-fno-strict-aliasing -I../../src/interfaces/libpq -I. -I../../src/include -I./src/include/port/win32
-DEXEC_BACKEND "-I../../src/include/port/win32" -c -o pgbench.o pgbench.c
gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Winline -Wdeclaration-after-statement -Wendif-labels
-fno-strict-aliasing
pgbench.o -L../../src/port -lpgport -L../../src/interfaces/libpq -lpq -L../../src/port -Wl,--allow-multiple-definition
-lpgport -lm -lws2_32 -lshfolder -o pgbench

I put in in order to avoid -D of the Makefile.

Regards,
Hiroshi Saito

"Hiroshi Saito" <z-saito@guitar.ocn.ne.jp> writes:

Why do you need to #undef EXEC_BACKEND, and is there a specific reason for
removing the include of win32.h?

I put in in order to avoid -D of the Makefile.

If that matters, the problem is that somebody put the wrong stuff in the
wrong include file. Backend-only things ought to go in postgres.h not
c.h. In particular this is wrongly placed:

/* EXEC_BACKEND defines */
#ifdef EXEC_BACKEND
#define NON_EXEC_STATIC
#else
#define NON_EXEC_STATIC static
#endif

but AFAICS it doesn't affect anything that pgbench would care about.
So I'm wondering *exactly* what goes wrong if you don't #undef
EXEC_BACKEND in pgbench.

As for the FRONTEND #define, that seems essential on Windows (and on no
other platform) because port/win32.h uses it. But putting the #define
into pgbench.c (and by implication into anything else we build on
Windows) sure seems like a broken approach. Where else could we put it?
It looks like right now that's left to the build system, which might or
might not be a good idea, but if it is a good idea then pgbench must be
getting missed. Perhaps instead postgres_fe.h should #define FRONTEND?

regards, tom lane

Hi.

From: "Tom Lane" <tgl@sss.pgh.pa.us>

"Hiroshi Saito" <z-saito@guitar.ocn.ne.jp> writes:

Why do you need to #undef EXEC_BACKEND, and is there a specific reason for
removing the include of win32.h?

I put in in order to avoid -D of the Makefile.

If that matters, the problem is that somebody put the wrong stuff in the
wrong include file. Backend-only things ought to go in postgres.h not
c.h. In particular this is wrongly placed:

/* EXEC_BACKEND defines */
#ifdef EXEC_BACKEND
#define NON_EXEC_STATIC
#else
#define NON_EXEC_STATIC static
#endif

but AFAICS it doesn't affect anything that pgbench would care about.
So I'm wondering *exactly* what goes wrong if you don't #undef
EXEC_BACKEND in pgbench.

As for the FRONTEND #define, that seems essential on Windows (and on no
other platform) because port/win32.h uses it. But putting the #define
into pgbench.c (and by implication into anything else we build on
Windows) sure seems like a broken approach. Where else could we put it?
It looks like right now that's left to the build system, which might or
might not be a good idea, but if it is a good idea then pgbench must be
getting missed. Perhaps instead postgres_fe.h should #define FRONTEND?

Yes, I feared that the physique of a main part broke. Then, Magnus said the
same suggestion as you. It seems that it needs to be brushup.
I am going to improve by the reason referred to as that FRONTEND influences
nmake (libpq) again. Probably, Mugnus is operating main part.?

Thanks.

Regards,
Hiroshi Saito

"Hiroshi Saito" <z-saito@guitar.ocn.ne.jp> writes:

From: "Tom Lane" <tgl@sss.pgh.pa.us>

Perhaps instead postgres_fe.h should #define FRONTEND?

Yes, I feared that the physique of a main part broke. Then, Magnus said the
same suggestion as you. It seems that it needs to be brushup.
I am going to improve by the reason referred to as that FRONTEND influences
nmake (libpq) again. Probably, Mugnus is operating main part.?

To be clear: my thought is to put "#define FRONTEND 1" into
postgres_fe.h and then eliminate ad-hoc definitions of it from a
boatload of Makefiles. A quick grep suggests that the only place this
wouldn't work too well is src/port/, but that could keep on doing what
it's doing.

I'm not in a good position to test this, because it will mainly matter
on Windows which I don't do. Anyone else have a chance to try it?

regards, tom lane

Hi.

From: "Tom Lane" <tgl@sss.pgh.pa.us>

"Hiroshi Saito" <z-saito@guitar.ocn.ne.jp> writes:

From: "Tom Lane" <tgl@sss.pgh.pa.us>

Perhaps instead postgres_fe.h should #define FRONTEND?

Yes, I feared that the physique of a main part broke. Then, Magnus said the
same suggestion as you. It seems that it needs to be brushup.
I am going to improve by the reason referred to as that FRONTEND influences
nmake (libpq) again. Probably, Mugnus is operating main part.?

To be clear: my thought is to put "#define FRONTEND 1" into
postgres_fe.h and then eliminate ad-hoc definitions of it from a
boatload of Makefiles. A quick grep suggests that the only place this
wouldn't work too well is src/port/, but that could keep on doing what
it's doing.

I think sufficient suggestion.

I'm not in a good position to test this, because it will mainly matter
on Windows which I don't do. Anyone else have a chance to try it?

I want, will try it. of course, work of Magnus is not barred.
Thanks!

Regards,
Hiroshi Saito

Hi.

To be clear: my thought is to put "#define FRONTEND 1" into
postgres_fe.h and then eliminate ad-hoc definitions of it from a
boatload of Makefiles. A quick grep suggests that the only place this
wouldn't work too well is src/port/, but that could keep on doing what
it's doing.

I tried it. and this is patch of test. Then, I still think that consideration
is required to a slight degree involving port.

However, This fully satisfied the following tests. (regression test is all pass)

FreeBSD
MinGW(gcc)
MS-VC8

What do you think?

Regards,
Hiroshi Saito

On Thu, Sep 27, 2007 at 03:21:59PM +0900, Hiroshi Saito wrote:

Hi.

To be clear: my thought is to put "#define FRONTEND 1" into
postgres_fe.h and then eliminate ad-hoc definitions of it from a
boatload of Makefiles. A quick grep suggests that the only place this
wouldn't work too well is src/port/, but that could keep on doing what
it's doing.

I tried it. and this is patch of test. Then, I still think that
consideration is required to a slight degree involving port.

However, This fully satisfied the following tests. (regression test is all
pass)

FreeBSD
MinGW(gcc)
MS-VC8

What do you think?

I will be offline for most of the time for a couple of days, so it will
probably be until early next week before I can look at this. Just a FYI,
but I'll be happy to look at it as soon as I can.

//Magnus

Magnus Hagander <magnus@hagander.net> writes:

On Thu, Sep 27, 2007 at 03:21:59PM +0900, Hiroshi Saito wrote:

What do you think?

I will be offline for most of the time for a couple of days, so it will
probably be until early next week before I can look at this. Just a FYI,
but I'll be happy to look at it as soon as I can.

I like the FRONTEND solution, but not the EXEC_BACKEND stuff --- my
objection there is that this formulation hard-wires EXEC_BACKEND to get
defined only on a WIN32 build, which complicates testing that code on
other platforms. (The whole point of the separate EXEC_BACKEND #define
was to let non-Windows developers test that code path, remember.)

My feeling is that we should continue to have EXEC_BACKEND driven by
CPPFLAGS, since that's easily tweaked on all platforms.

I'm still not clear on why anything needs to be done with
NON_EXEC_STATIC --- AFAICS that symbol is only referenced in half
a dozen backend-only .c files, so I think we can just leave it as
it stands.

In the interests of pushing 8.3beta forward, I'm going to go ahead
and commit this patch with the above mods; the buildfarm will let
us know if there's anything seriously wrong ...

regards, tom lane

Magnus Hagander <magnus@hagander.net> writes:

On Tue, Aug 07, 2007 at 04:58:19PM +0900, Hiroshi Saito wrote:

pgbench.c: In function `main':
pgbench.c:1257: warning: implicit declaration of function `getopt'

adjustment of some reference is required for this.

Why do you need to #undef EXEC_BACKEND, and is there a specific reason for
removing the include of win32.h?

I've applied the attached modified version of this patch; it keeps
win32.h in there (correctly re-marked as a system header). I think
the fundamental issue must be that Hiroshi's system sets HAVE_GETOPT and
HAVE_GETOPT_H. The former causes port.h to not supply a declaration
of getopt(), so we'd better include <getopt.h> to declare it.

Together with the changes for FRONTEND applied a little bit ago,
I hope that this issue is fully resolved in CVS HEAD. Let me know if
there's still a problem.

regards, tom lane

Hi.

----- Original Message -----
From: "Tom Lane" <tgl@sss.pgh.pa.us>

Magnus Hagander <magnus@hagander.net> writes:

On Thu, Sep 27, 2007 at 03:21:59PM +0900, Hiroshi Saito wrote:

What do you think?

I will be offline for most of the time for a couple of days, so it will
probably be until early next week before I can look at this. Just a FYI,
but I'll be happy to look at it as soon as I can.

I like the FRONTEND solution, but not the EXEC_BACKEND stuff --- my
objection there is that this formulation hard-wires EXEC_BACKEND to get
defined only on a WIN32 build, which complicates testing that code on
other platforms. (The whole point of the separate EXEC_BACKEND #define
was to let non-Windows developers test that code path, remember.)

Ah yes, I also worried that a little change might affect other platforms by
the complexity of the action. we memorable it..

My feeling is that we should continue to have EXEC_BACKEND driven by
CPPFLAGS, since that's easily tweaked on all platforms.

I'm still not clear on why anything needs to be done with
NON_EXEC_STATIC --- AFAICS that symbol is only referenced in half
a dozen backend-only .c files, so I think we can just leave it as
it stands.

Although I am attached by the reason it happen the problem in a reference
relation by windows, main() which it is called thinks in original that it is good
by "non static". I look at that "non static ..main()" fully operates by FreeBSD.
Does it influence performance?

In the interests of pushing 8.3beta forward, I'm going to go ahead
and commit this patch with the above mods; the buildfarm will let
us know if there's anything seriously wrong ...

Yeah, since it becomes better. thanks!

Regards,
Hiroshi Saito

Hi.

From: "Tom Lane" <tgl@sss.pgh.pa.us>

Magnus Hagander <magnus@hagander.net> writes:

On Tue, Aug 07, 2007 at 04:58:19PM +0900, Hiroshi Saito wrote:

pgbench.c: In function `main':
pgbench.c:1257: warning: implicit declaration of function `getopt'

adjustment of some reference is required for this.

Why do you need to #undef EXEC_BACKEND, and is there a specific reason for
removing the include of win32.h?

I've applied the attached modified version of this patch; it keeps
win32.h in there (correctly re-marked as a system header). I think
the fundamental issue must be that Hiroshi's system sets HAVE_GETOPT and
HAVE_GETOPT_H. The former causes port.h to not supply a declaration
of getopt(), so we'd better include <getopt.h> to declare it.

Together with the changes for FRONTEND applied a little bit ago,
I hope that this issue is fully resolved in CVS HEAD. Let me know if
there's still a problem.

Yeah, I think that it felt it very refreshed.!:-)
Though FRONTEND needs adjustment to consider carefully.
Thanks.

Regards,
Hiroshi Saito