Andrew Dunstan wrote:
> 
> >this code will fail rather horribly if sizeof(pid_t) != sizeof(int).
> >Even more to the point, I believe a standalone backend will put
> >the negative of its PID into the file, and the revised code will fail
> >to parse that at all.
> >
> >I think the safest code would be like
> >
> >     long    tmp;
> >
> >     fscanf(pidf, "%ld", &tmp);
> >     if (tmp < 0)
> >     {
> >             tmp = -tmp;
> >             // do anything else needed for backend case
> >     }
> >     pid = (pid_t) tmp;
> >
> >
> >  
> >
> 
> I deliberately used a signed long for these reasons in the first place.
> 
> The number of places we actually need to use this value as a pid is 
> small (3 by my count - in calls to kill() ), and it was cast there to 
> pid_t,  I think.  I still don't see what's wrong with that.

OK, I created a typedef pgpid_t equal to long and used that in the code,
change format to %ld, and documented why negative values are an issue.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  [EMAIL PROTECTED]               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
? pg_ctl
Index: pg_ctl.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/bin/pg_ctl/pg_ctl.c,v
retrieving revision 1.2
diff -c -c -r1.2 pg_ctl.c
*** pg_ctl.c    31 May 2004 17:57:31 -0000      1.2
--- pg_ctl.c    1 Jun 2004 01:26:40 -0000
***************
*** 24,29 ****
--- 24,31 ----
  int                   optreset;
  #endif
  
+ /* PID can be negative for standalone backend */
+ typedef long pgpid_t;
  
  #define _(x) gettext((x))
  
***************
*** 79,86 ****
  static void do_restart(void);
  static void do_reload(void);
  static void do_status(void);
! static void do_kill(pid_t pid);
! static pid_t get_pgpid(void);
  static char **readfile(char *path);
  static int start_postmaster(void);
  static bool test_postmaster_connection(void);
--- 81,88 ----
  static void do_restart(void);
  static void do_reload(void);
  static void do_status(void);
! static void do_kill(pgpid_t pid);
! static pgpid_t get_pgpid(void);
  static char **readfile(char *path);
  static int start_postmaster(void);
  static bool test_postmaster_connection(void);
***************
*** 126,136 ****
  
  
  
! static pid_t
  get_pgpid(void)
  {
        FILE       *pidf;
!       pid_t           pid;
  
        pidf = fopen(pid_file, "r");
        if (pidf == NULL)
--- 128,138 ----
  
  
  
! static pgpid_t
  get_pgpid(void)
  {
        FILE       *pidf;
!       pgpid_t         pid;
  
        pidf = fopen(pid_file, "r");
        if (pidf == NULL)
***************
*** 144,150 ****
                        exit(1);
                }
        }
!       fscanf(pidf, "%u", &pid);
        fclose(pidf);
        return pid;
  }
--- 146,152 ----
                        exit(1);
                }
        }
!       fscanf(pidf, "%ld", &pid);
        fclose(pidf);
        return pid;
  }
***************
*** 323,330 ****
  static void
  do_start(void)
  {
!       pid_t           pid;
!       pid_t           old_pid = 0;
        char       *optline = NULL;
  
        if (ctl_command != RESTART_COMMAND)
--- 325,332 ----
  static void
  do_start(void)
  {
!       pgpid_t         pid;
!       pgpid_t         old_pid = 0;
        char       *optline = NULL;
  
        if (ctl_command != RESTART_COMMAND)
***************
*** 457,463 ****
  do_stop(void)
  {
        int                     cnt;
!       pid_t           pid;
  
        pid = get_pgpid();
  
--- 459,465 ----
  do_stop(void)
  {
        int                     cnt;
!       pgpid_t         pid;
  
        pid = get_pgpid();
  
***************
*** 472,485 ****
                pid = -pid;
                fprintf(stderr,
                                _("%s: cannot stop postmaster; "
!                               "postgres is running (PID: %u)\n"),
                                progname, pid);
                exit(1);
        }
  
!       if (kill(pid, sig) != 0)
        {
!               fprintf(stderr, _("stop signal failed (PID: %u): %s\n"), pid,
                                strerror(errno));
                exit(1);
        }
--- 474,487 ----
                pid = -pid;
                fprintf(stderr,
                                _("%s: cannot stop postmaster; "
!                               "postgres is running (PID: %ld)\n"),
                                progname, pid);
                exit(1);
        }
  
!       if (kill((pid_t) pid, sig) != 0)
        {
!               fprintf(stderr, _("stop signal failed (PID: %ld): %s\n"), pid,
                                strerror(errno));
                exit(1);
        }
***************
*** 537,543 ****
  do_restart(void)
  {
        int                     cnt;
!       pid_t           pid;
  
        pid = get_pgpid();
  
--- 539,545 ----
  do_restart(void)
  {
        int                     cnt;
!       pgpid_t         pid;
  
        pid = get_pgpid();
  
***************
*** 553,567 ****
                pid = -pid;
                fprintf(stderr,
                                _("%s: cannot restart postmaster; "
!                               "postgres is running (PID: %u)\n"),
                                progname, pid);
                fprintf(stderr, _("Please terminate postgres and try again.\n"));
                exit(1);
        }
  
!       if (kill(pid, sig) != 0)
        {
!               fprintf(stderr, _("stop signal failed (PID: %u): %s\n"), pid,
                                strerror(errno));
                exit(1);
        }
--- 555,569 ----
                pid = -pid;
                fprintf(stderr,
                                _("%s: cannot restart postmaster; "
!                               "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)
        {
!               fprintf(stderr, _("stop signal failed (PID: %ld): %s\n"), pid,
                                strerror(errno));
                exit(1);
        }
***************
*** 609,615 ****
  static void
  do_reload(void)
  {
!       pid_t           pid;
  
        pid = get_pgpid();
        if (pid == 0)                           /* no pid file */
--- 611,617 ----
  static void
  do_reload(void)
  {
!       pgpid_t         pid;
  
        pid = get_pgpid();
        if (pid == 0)                           /* no pid file */
***************
*** 623,637 ****
                pid = -pid;
                fprintf(stderr,
                                _("%s: cannot reload postmaster; "
!                               "postgres is running (PID: %u)\n"),
                                progname, pid);
                fprintf(stderr, _("Please terminate postgres and try again.\n"));
                exit(1);
        }
  
!       if (kill(pid, sig) != 0)
        {
!               fprintf(stderr, _("reload signal failed (PID: %u): %s\n"), pid,
                                strerror(errno));
                exit(1);
        }
--- 625,639 ----
                pid = -pid;
                fprintf(stderr,
                                _("%s: cannot reload postmaster; "
!                               "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)
        {
!               fprintf(stderr, _("reload signal failed (PID: %ld): %s\n"), pid,
                                strerror(errno));
                exit(1);
        }
***************
*** 647,653 ****
  static void
  do_status(void)
  {
!       pid_t           pid;
  
        pid = get_pgpid();
        if (pid == 0)                           /* no pid file */
--- 649,655 ----
  static void
  do_status(void)
  {
!       pgpid_t         pid;
  
        pid = get_pgpid();
        if (pid == 0)                           /* no pid file */
***************
*** 658,670 ****
        else if (pid < 0)                       /* standalone backend */
        {
                pid = -pid;
!               fprintf(stdout, _("%s: a standalone backend \"postgres\" is running 
(PID: %u)\n"), progname, pid);
        }
        else                                            /* postmaster */
        {
                char      **optlines;
  
!               fprintf(stdout, _("%s: postmaster is running (PID: %u)\n"), progname, 
pid);
  
                optlines = readfile(postopts_file);
                if (optlines != NULL)
--- 660,672 ----
        else if (pid < 0)                       /* standalone backend */
        {
                pid = -pid;
!               fprintf(stdout, _("%s: a standalone backend \"postgres\" is running 
(PID: %ld)\n"), progname, pid);
        }
        else                                            /* postmaster */
        {
                char      **optlines;
  
!               fprintf(stdout, _("%s: postmaster is running (PID: %ld)\n"), progname, 
pid);
  
                optlines = readfile(postopts_file);
                if (optlines != NULL)
***************
*** 676,686 ****
  
  
  static void
! do_kill(pid_t pid)
  {
!       if (kill(pid, sig) != 0)
        {
!               fprintf(stderr, _("signal %d failed (PID: %u): %s\n"), sig, pid,
                                strerror(errno));
                exit(1);
        }
--- 678,688 ----
  
  
  static void
! do_kill(pgpid_t pid)
  {
!       if (kill((pid_t) pid, sig) != 0)
        {
!               fprintf(stderr, _("signal %d failed (PID: %ld): %s\n"), sig, pid,
                                strerror(errno));
                exit(1);
        }
***************
*** 813,819 ****
  
        int                     option_index;
        int                     c;
!       int                     killproc = 0;
        
  #ifdef WIN32
        setvbuf(stderr, NULL, _IONBF, 0);
--- 815,821 ----
  
        int                     option_index;
        int                     c;
!       pgpid_t         killproc = 0;
        
  #ifdef WIN32
        setvbuf(stderr, NULL, _IONBF, 0);
---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

               http://archives.postgresql.org

Reply via email to