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.)


however below the result of my quich review:

1) exit(1)  => exit(EXIT_FAILURE)
2) xstrdup protected by duplicate NULL string

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


Regards Gaetano Mendola






*** pg_ctl.c 2004-05-26 02:48:38.000000000 +0200 --- pg_ctl.c.orig 2004-05-26 02:43:32.000000000 +0200 *************** *** 26,33 **** /* postmaster version ident string */ #define PM_VERSIONSTR "postmaster (PostgreSQL) " PG_VERSION "\n"

- #define EXIT_FAILURE 1
-

  typedef enum
  {
--- 26,31 ----
***************
*** 100,106 ****
        if (!result)
        {
                fprintf(stderr, _("%s: out of memory\n"), progname);
!               exit(EXIT_FAILURE);
        }
        return result;
  }
--- 98,104 ----
        if (!result)
        {
                fprintf(stderr, _("%s: out of memory\n"), progname);
!               exit(1);
        }
        return result;
  }
***************
*** 112,130 ****
  {
        char       *result;

-
-       if (!s)
-       {
-               fprintf(stderr, "%s: can not duplicate null pointer", progname);
-               exit(EXIT_FAILURE);
-       }
-
-
        result = strdup(s);
        if (!result)
        {
                fprintf(stderr, _("%s: out of memory\n"), progname);
!               exit(EXIT_FAILURE);
        }
        return result;
  }
--- 110,120 ----
  {
        char       *result;

        result = strdup(s);
        if (!result)
        {
                fprintf(stderr, _("%s: out of memory\n"), progname);
!               exit(1);
        }
        return result;
  }
***************
*** 146,152 ****
                else
                {
                        perror("openning pid file");
!                       exit(EXIT_FAILURE);
                }
        }
        fscanf(pidf, "%ld", &pid);
--- 136,142 ----
                else
                {
                        perror("openning pid file");
!                       exit(1);
                }
        }
        fscanf(pidf, "%ld", &pid);
***************
*** 353,359 ****
                        else
                        {
                                fprintf(stderr, "%s: cannot read %s\n", progname, 
postopts_file);
!                               exit(EXIT_FAILURE);
                        }
                }
                else if (optlines[0] == NULL || optlines[1] != NULL)
--- 343,349 ----
                        else
                        {
                                fprintf(stderr, "%s: cannot read %s\n", progname, 
postopts_file);
!                               exit(1);
                        }
                }
                else if (optlines[0] == NULL || optlines[1] != NULL)
***************
*** 361,367 ****
                        fprintf(stderr, "%s: option file %s must have exactly 1 
line\n",
                                        progname, ctl_command == RESTART_COMMAND ?
                                        postopts_file : def_postopts_file);
!                       exit(EXIT_FAILURE);
                }
                else
                {
--- 351,357 ----
                        fprintf(stderr, "%s: option file %s must have exactly 1 
line\n",
                                        progname, ctl_command == RESTART_COMMAND ?
                                        postopts_file : def_postopts_file);
!                       exit(1);
                }
                else
                {
***************
*** 411,417 ****
                                                  "but was not the same version as 
\"%s\".\n"
                                                  "Check your installation.\n"),
                                                progname, progname);
!                       exit(EXIT_FAILURE);
                }
                postgres_path = postmaster_path;
        }
--- 401,407 ----
                                                  "but was not the same version as 
\"%s\".\n"
                                                  "Check your installation.\n"),
                                                progname, progname);
!                       exit(1);
                }
                postgres_path = postmaster_path;
        }
***************
*** 428,434 ****
                                        "%s: cannot start postmaster\n"
                                        "Examine the log output\n",
                                        progname);
!                       exit(EXIT_FAILURE);
                }
        }

--- 418,424 ----
                                        "%s: cannot start postmaster\n"
                                        "Examine the log output\n",
                                        progname);
!                       exit(1);
                }
        }

***************
*** 463,469 ****
        {
                fprintf(stderr, "%s: could not find %s\n", progname, pid_file);
                fprintf(stderr, "Is postmaster running?\n");
!               exit(EXIT_FAILURE);
        }
        else if (pid < 0)                    /* standalone backend, not postmaster */
        {
--- 453,459 ----
        {
                fprintf(stderr, "%s: could not find %s\n", progname, pid_file);
                fprintf(stderr, "Is postmaster running?\n");
!               exit(1);
        }
        else if (pid < 0)                    /* standalone backend, not postmaster */
        {
***************
*** 472,478 ****
                                "%s: cannot stop postmaster; "
                                "postgres is running (PID: %ld)\n",
                                progname, pid);
!               exit(EXIT_FAILURE);
        }

        if (kill((pid_t) pid, sig) != 0)
--- 462,468 ----
                                "%s: cannot stop postmaster; "
                                "postgres is running (PID: %ld)\n",
                                progname, pid);
!               exit(1);
        }

        if (kill((pid_t) pid, sig) != 0)
***************
*** 513,519 ****
                                printf(" failed\n");
        
                        fprintf(stderr, "%s: postmaster does not shut down\n", 
progname);
!                       exit(EXIT_FAILURE);
                }
        }
  }
--- 503,509 ----
                                printf(" failed\n");
        
                        fprintf(stderr, "%s: postmaster does not shut down\n", 
progname);
!                       exit(1);
                }
        }
  }
***************
*** 546,552 ****
                                "postgres is running (PID: %ld)\n",
                                progname, pid);
                fprintf(stderr, "Please terminate postgres and try again.\n");
!               exit(EXIT_FAILURE);
        }

        if (kill((pid_t) pid, sig) != 0)
--- 536,542 ----
                                "postgres is running (PID: %ld)\n",
                                progname, pid);
                fprintf(stderr, "Please terminate postgres and try again.\n");
!               exit(1);
        }

        if (kill((pid_t) pid, sig) != 0)
***************
*** 581,587 ****
                        printf(" failed\n");

                fprintf(stderr, "%s: postmaster does not shut down\n", progname);
!               exit(EXIT_FAILURE);
        }

        do_start();
--- 571,577 ----
                        printf(" failed\n");

                fprintf(stderr, "%s: postmaster does not shut down\n", progname);
!               exit(1);
        }

        do_start();
***************
*** 599,605 ****
        {
                fprintf(stderr, "%s: could not find %s\n", progname, pid_file);
                fprintf(stderr, "Is postmaster running?\n");
!               exit(EXIT_FAILURE);
        }
        else if (pid < 0)                    /* standalone backend, not postmaster */
        {
--- 589,595 ----
        {
                fprintf(stderr, "%s: could not find %s\n", progname, pid_file);
                fprintf(stderr, "Is postmaster running?\n");
!               exit(1);
        }
        else if (pid < 0)                    /* standalone backend, not postmaster */
        {
***************
*** 609,615 ****
                                "postgres is running (PID: %ld)\n",
                                progname, pid);
                fprintf(stderr, "Please terminate postgres and try again.\n");
!               exit(EXIT_FAILURE);
        }

        if (kill((pid_t) pid, sig) != 0)
--- 599,605 ----
                                "postgres is running (PID: %ld)\n",
                                progname, pid);
                fprintf(stderr, "Please terminate postgres and try again.\n");
!               exit(1);
        }

        if (kill((pid_t) pid, sig) != 0)
***************
*** 632,638 ****
        if (pid == 0)                           /* no pid file */
        {
                fprintf(stderr, "%s: postmaster or postgres not running", progname);
!               exit(EXIT_FAILURE);
        }
        else if (pid < 0)                    /* standalone backend */
        {
--- 622,628 ----
        if (pid == 0)                           /* no pid file */
        {
                fprintf(stderr, "%s: postmaster or postgres not running", progname);
!               exit(1);
        }
        else if (pid < 0)                    /* standalone backend */
        {
***************
*** 745,751 ****
        {
                fprintf(stderr, "%s: invalid shutdown mode %s\n", progname, modeopt);
                do_advice();
!               exit(EXIT_FAILURE);
        }
  }

--- 735,741 ----
        {
                fprintf(stderr, "%s: invalid shutdown mode %s\n", progname, modeopt);
                do_advice();
!               exit(1);
        }
  }

***************
*** 778,784 ****
        {
                fprintf(stderr, "%s: invalid signal \"%s\"\n", progname, signame);
                do_advice();
!               exit(EXIT_FAILURE);
        }

  }
--- 768,774 ----
        {
                fprintf(stderr, "%s: invalid signal \"%s\"\n", progname, signame);
                do_advice();
!               exit(1);
        }

  }
***************
*** 878,884 ****
                {
                        fprintf(stderr, "%s: invalid option %s\n", progname, arg);
                        do_advice();
!                       exit(EXIT_FAILURE);
                }
                else if (strcmp(arg, "start") == 0 && ctl_command == NO_COMMAND)
                        ctl_command = START_COMMAND;
--- 868,874 ----
                {
                        fprintf(stderr, "%s: invalid option %s\n", progname, arg);
                        do_advice();
!                       exit(1);
                }
                else if (strcmp(arg, "start") == 0 && ctl_command == NO_COMMAND)
                        ctl_command = START_COMMAND;
***************
*** 902,908 ****
                {
                        fprintf(stderr, "%s: invalid operation mode %s\n", progname, 
arg);
                        do_advice();
!                       exit(EXIT_FAILURE);
                }
        }

--- 892,898 ----
                {
                        fprintf(stderr, "%s: invalid operation mode %s\n", progname, 
arg);
                        do_advice();
!                       exit(1);
                }
        }

***************
*** 910,916 ****
        {
                fprintf(stderr, "%s: no operation specified\n", progname);
                do_advice();
!               exit(EXIT_FAILURE);
        }

        pg_data = getenv("PGDATA");
--- 900,906 ----
        {
                fprintf(stderr, "%s: no operation specified\n", progname);
                do_advice();
!               exit(1);
        }

        pg_data = getenv("PGDATA");
***************
*** 923,929 ****
                                "and environment variable PGDATA unset\n",
                                progname);
                do_advice();
!               exit(EXIT_FAILURE);
        }

        if (!wait_set)
--- 913,919 ----
                                "and environment variable PGDATA unset\n",
                                progname);
                do_advice();
!               exit(1);
        }

        if (!wait_set)




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

Reply via email to