Обсуждение: Warning is adjusted of pgbench.
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). Regards, Hiroshi Saito
Вложения
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 > 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
"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
Index: pgbench.c
===================================================================
RCS file: /cvsroot/pgsql/contrib/pgbench/pgbench.c,v
retrieving revision 1.71
retrieving revision 1.72
diff -c -r1.71 -r1.72
*** pgbench.c 25 Aug 2007 09:21:14 -0000 1.71
--- pgbench.c 27 Sep 2007 20:39:43 -0000 1.72
***************
*** 24,33 ****
#include <ctype.h>
#ifdef WIN32
! #include "win32.h"
#else
#include <sys/time.h>
#include <unistd.h>
#ifdef HAVE_GETOPT_H
#include <getopt.h>
--- 24,34 ----
#include <ctype.h>
#ifdef WIN32
! #include <win32.h>
#else
#include <sys/time.h>
#include <unistd.h>
+ #endif /* ! WIN32 */
#ifdef HAVE_GETOPT_H
#include <getopt.h>
***************
*** 40,54 ****
#ifdef HAVE_SYS_RESOURCE_H
#include <sys/resource.h> /* for getrlimit */
#endif
- #endif /* ! WIN32 */
extern char *optarg;
extern int optind;
- #ifdef WIN32
- #undef select
- #endif
-
/********************************************************************
* some configurable parameters */
--- 41,50 ----
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