Skip site navigation (1) Skip section navigation (2)

Re: pg_ctl.c

From: Gaetano Mendola <mendola(at)bigfoot(dot)com>
To: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
Subject: Re: pg_ctl.c
Date: 2004-05-26 01:04:35
Message-ID: 40B3ED23.1060507@bigfoot.com (view raw or flat)
Thread:
Lists: pgsql-patches
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)




In response to

  • pg_ctl.c at 2004-05-25 20:57:01 from Bruce Momjian

Responses

pgsql-patches by date

Next:From: Neil ConwayDate: 2004-05-26 04:44:43
Subject: final list rewrite patch
Previous:From: Bruce MomjianDate: 2004-05-26 00:14:57
Subject: Re: contrib/dbmirror typo fix

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group