On Mon, Nov  4, 2013 at 11:31:30AM -0500, Robert Haas wrote:
> On Sat, Nov 2, 2013 at 3:32 PM, Peter Eisentraut <pete...@gmx.net> wrote:
> > This doesn't seem right:
> >
> > $ pg_ctl -D /nowhere status
> > pg_ctl: no server running
> >
> > It does exit with status 3, so it's not all that broken, but I think the
> > error message could be more accurate.
> 
> I doubt anyone will object if you feel the urge to fix it.  I won't, anyway.

I have addressed this issue with the attached patch:

        $ pg_ctl -D /lkjasdf status
        pg_ctl: directory "/lkjasdf" does not exist
        $ pg_ctl -D / status
        pg_ctl: directory "/" is not a database cluster directory

One odd question is that pg_ctl status has this comment for reporting
the exit code for non-running servers:

    printf(_("%s: no server running\n"), progname);

    /*
     * The Linux Standard Base Core Specification 3.1 says this should return
     * '3'
     * 
https://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB-Core-generic/iniscrptact.html
     */
    exit(3);

If they haven't passed us a data directory, we don't really know if the
server is running or not, so the patch just returns '1'.


-- 
  Bruce Momjian  <br...@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
new file mode 100644
index 0dbaa6e..90c98ee
*** a/src/bin/pg_ctl/pg_ctl.c
--- b/src/bin/pg_ctl/pg_ctl.c
*************** static bool allow_core_files = false;
*** 97,102 ****
--- 97,103 ----
  static time_t start_time;
  
  static char postopts_file[MAXPGPATH];
+ static char version_file[MAXPGPATH];
  static char pid_file[MAXPGPATH];
  static char backup_file[MAXPGPATH];
  static char recovery_file[MAXPGPATH];
*************** get_pgpid(void)
*** 250,255 ****
--- 251,275 ----
  {
  	FILE	   *pidf;
  	long		pid;
+ 	struct stat statbuf;
+ 
+ 	if (stat(pg_data, &statbuf) != 0)
+ 	{
+ 		if (errno == ENOENT)
+ 			printf(_("%s: directory \"%s\" does not exist\n"), progname,
+ 					 pg_data);
+ 		else
+ 			printf(_("%s: cannot access directory \"%s\"\n"), progname,
+ 					 pg_data);
+ 		exit(1);
+ 	}
+ 
+ 	if (stat(version_file, &statbuf) != 0 && errno == ENOENT)
+ 	{
+ 		printf(_("%s: directory \"%s\" is not a database cluster directory\n"),
+ 				 progname, pg_data);
+ 		exit(1);
+ 	}
  
  	pidf = fopen(pid_file, "r");
  	if (pidf == NULL)
*************** main(int argc, char **argv)
*** 2285,2290 ****
--- 2305,2311 ----
  	if (pg_data)
  	{
  		snprintf(postopts_file, MAXPGPATH, "%s/postmaster.opts", pg_data);
+ 		snprintf(version_file, MAXPGPATH, "%s/PG_VERSION", pg_data);
  		snprintf(pid_file, MAXPGPATH, "%s/postmaster.pid", pg_data);
  		snprintf(backup_file, MAXPGPATH, "%s/backup_label", pg_data);
  		snprintf(recovery_file, MAXPGPATH, "%s/recovery.conf", pg_data);
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to