Gaetano Mendola wrote:

Bruce Momjian wrote:

Here is the C version of pg_ctl.c written by Andrew Dunstan and updated
by me.

You can use it by creating a src/bin/pg_ctl_test directory and putting
the C and Makefile into that directory.  You can then do a make install
and use it for testing.

Unless someone finds a problem, I will apply the change soon.  This
removes our last shell script!


It desn't compile on my platform:

$ gcc -I /usr/include/pgsql/server pg_ctl.c
pg_ctl.c: In function `start_postmaster':
pg_ctl.c:219: error: `DEVNULL' undeclared (first use in this function)
pg_ctl.c:219: error: (Each undeclared identifier is reported only once
pg_ctl.c:219: error: for each function it appears in.)


It does not appear that you have followed Bruce's instructions above for testing this. It works just fine for me:

[EMAIL PROTECTED] pg_ctl_x]$ make
make -C ../../../src/interfaces/libpq all
make[1]: Entering directory `/home/andrew/pgwnew/pgsql/src/interfaces/libpq'
make[1]: Nothing to be done for `all'.
make[1]: Leaving directory `/home/andrew/pgwnew/pgsql/src/interfaces/libpq'
make -C ../../../src/port all
make[1]: Entering directory `/home/andrew/pgwnew/pgsql/src/port'
make[1]: Nothing to be done for `all'.
make[1]: Leaving directory `/home/andrew/pgwnew/pgsql/src/port'
gcc -O2 -fno-strict-aliasing -Wall -Wmissing-prototypes -Wmissing-declarations -DFRONTEND -DDEF_PGPORT=5432 -I../../../src/interfaces/libpq -I../../../src/include -D_GNU_SOURCE -c -o pg_ctl.o pg_ctl.c
rm -f exec.c && ln -s ../../../src/port/exec.c .
gcc -O2 -fno-strict-aliasing -Wall -Wmissing-prototypes -Wmissing-declarations -DFRONTEND -DDEF_PGPORT=5432 -I../../../src/interfaces/libpq -I../../../src/include -D_GNU_SOURCE -c -o exec.o exec.c
gcc -O2 -fno-strict-aliasing -Wall -Wmissing-prototypes -Wmissing-declarations pg_ctl.o exec.o -L../../../src/interfaces/libpq -lpq -L../../../src/port -Wl,-rpath,/usr/local/pgsql/lib -lz -lreadline -ltermcap -lcrypt -lresolv -lnsl -ldl -lm -lbsd -lpgport -o pg_ctl
[EMAIL PROTECTED] pg_ctl_x]$



What version of the pg include files is in /usr/include/pgsql/server ? If <= 7.4 then of course DEVNULL will not be defined.




however below the result of my quich review:

1) exit(1) => exit(EXIT_FAILURE)


If we used a number of different error codes I might agree. But it seems pointless here, and the style is widely used in our code base (I just counted 201 other occurrrences, not including cases of exit(0) ).

2) xstrdup protected by duplicate NULL string


I don't object, but it is redundant - in every case where it is called the argument is demonstrably not NULL.


I seen also that you don't use always the _ macro for error display.


True - that's part of the polish needed.

BTW, please don't send reverse diffs, they are a pain to read, IMNSHO (i.e. you should do diff -c file.c.orig file.c instead of having the files the other way around).

There is one small thing that is wrong with it - an incorrect format argument. see patch below.

cheers

andrew

*** pg_ctl.c.orig       2004-05-26 10:27:20.000000000 -0400
--- pg_ctl.c    2004-05-26 10:28:34.000000000 -0400
***************
*** 237,243 ****
       *portstr = '\0';

       if (getenv("PGPORT") != NULL)   /* environment */
!               snprintf(portstr, sizeof(portstr), "%d", getenv("PGPORT"));
       else    /* post_opts */
       {
               char    *p;
--- 237,243 ----
       *portstr = '\0';

       if (getenv("PGPORT") != NULL)   /* environment */
!               snprintf(portstr, sizeof(portstr), "%s", getenv("PGPORT"));
       else    /* post_opts */
       {
               char    *p;


---------------------------(end of broadcast)--------------------------- TIP 8: explain analyze is your friend

Reply via email to