Re: Refactoring code stripping trailing \n and \r from strings

2019-08-08 Thread Michael Paquier
On Wed, Aug 07, 2019 at 10:10:36AM +0900, Michael Paquier wrote:
> Thanks for the review, Bruce!  Tom, do you have any objections?

hearing nothing but cicadas from outside, applied.  There were some
warnings I missed with the first version, which are fixed.
--
Michael


signature.asc
Description: PGP signature


Re: Refactoring code stripping trailing \n and \r from strings

2019-08-06 Thread Michael Paquier
On Tue, Aug 06, 2019 at 03:10:33PM -0400, Bruce Momjian wrote:
> On Thu, Aug  1, 2019 at 12:18:20PM +0900, Michael Paquier wrote:
>> b654714 has reworked the way we handle removal of CLRF for several
>> code paths, and has repeated the same code patterns to do that in 8
>> different places.  Could it make sense to refactor things as per the
>> attached with a new routine in common/string.c?
> 
> Yes, I think this is a good idea.

Thanks for the review, Bruce!  Tom, do you have any objections?
--
Michael


signature.asc
Description: PGP signature


Re: Refactoring code stripping trailing \n and \r from strings

2019-08-06 Thread Bruce Momjian
On Thu, Aug  1, 2019 at 12:18:20PM +0900, Michael Paquier wrote:
> Hi Tom,
> 
> b654714 has reworked the way we handle removal of CLRF for several
> code paths, and has repeated the same code patterns to do that in 8
> different places.  Could it make sense to refactor things as per the
> attached with a new routine in common/string.c?

Yes, I think this is a good idea.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +




Refactoring code stripping trailing \n and \r from strings

2019-07-31 Thread Michael Paquier
Hi Tom,

b654714 has reworked the way we handle removal of CLRF for several
code paths, and has repeated the same code patterns to do that in 8
different places.  Could it make sense to refactor things as per the
attached with a new routine in common/string.c?

Thanks,
--
Michael
diff --git a/src/backend/libpq/be-secure-common.c b/src/backend/libpq/be-secure-common.c
index 4abbef5bf1..e8f27bc782 100644
--- a/src/backend/libpq/be-secure-common.c
+++ b/src/backend/libpq/be-secure-common.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 
+#include "common/string.h"
 #include "libpq/libpq.h"
 #include "storage/fd.h"
 
@@ -112,11 +113,8 @@ run_ssl_passphrase_command(const char *prompt, bool is_server_start, char *buf,
 		goto error;
 	}
 
-	/* strip trailing newline, including \r in case we're on Windows */
-	len = strlen(buf);
-	while (len > 0 && (buf[len - 1] == '\n' ||
-	   buf[len - 1] == '\r'))
-		buf[--len] = '\0';
+	/* strip trailing newline and carriage return */
+	len = pg_strip_crlf(buf);
 
 error:
 	pfree(command.data);
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 068b65a2af..6ef004f192 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -27,6 +27,7 @@
 #include "common/controldata_utils.h"
 #include "common/file_perm.h"
 #include "common/logging.h"
+#include "common/string.h"
 #include "getopt_long.h"
 #include "utils/pidfile.h"
 
@@ -2219,12 +2220,8 @@ adjust_data_dir(void)
 	pclose(fd);
 	free(my_exec_path);
 
-	/* Remove trailing newline, handling Windows newlines as well */
-	len = strlen(filename);
-	while (len > 0 &&
-		   (filename[len - 1] == '\n' ||
-			filename[len - 1] == '\r'))
-		filename[--len] = '\0';
+	/* strip trailing newline and carriage return */
+	len = pg_strip_crlf(filename);
 
 	free(pg_data);
 	pg_data = pg_strdup(filename);
diff --git a/src/bin/pg_resetwal/pg_resetwal.c b/src/bin/pg_resetwal/pg_resetwal.c
index 9d9c33d78c..702986539b 100644
--- a/src/bin/pg_resetwal/pg_resetwal.c
+++ b/src/bin/pg_resetwal/pg_resetwal.c
@@ -54,6 +54,7 @@
 #include "common/file_perm.h"
 #include "common/logging.h"
 #include "common/restricted_token.h"
+#include "common/string.h"
 #include "storage/large_object.h"
 #include "pg_getopt.h"
 #include "getopt_long.h"
@@ -557,12 +558,8 @@ CheckDataVersion(void)
 		exit(1);
 	}
 
-	/* remove trailing newline, handling Windows newlines as well */
-	len = strlen(rawline);
-	while (len > 0 &&
-		   (rawline[len - 1] == '\n' ||
-			rawline[len - 1] == '\r'))
-		rawline[--len] = '\0';
+	/* strip trailing newline and carriage return */
+	len = pg_strip_crlf(rawline);
 
 	if (strcmp(rawline, PG_MAJORVERSION) != 0)
 	{
diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index 7e3d3f1bb2..c74f05c032 100644
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -411,7 +411,6 @@ adjust_data_dir(ClusterInfo *cluster)
 cmd_output[MAX_STRING];
 	FILE	   *fp,
 			   *output;
-	int			len;
 
 	/* Initially assume config dir and data dir are the same */
 	cluster->pgconfig = pg_strdup(cluster->pgdata);
@@ -452,12 +451,8 @@ adjust_data_dir(ClusterInfo *cluster)
 
 	pclose(output);
 
-	/* Remove trailing newline, handling Windows newlines as well */
-	len = strlen(cmd_output);
-	while (len > 0 &&
-		   (cmd_output[len - 1] == '\n' ||
-			cmd_output[len - 1] == '\r'))
-		cmd_output[--len] = '\0';
+	/* strip trailing newline and carriage return */
+	(void) pg_strip_crlf(cmd_output);
 
 	cluster->pgdata = pg_strdup(cmd_output);
 
@@ -518,15 +513,9 @@ get_sock_dir(ClusterInfo *cluster, bool live_check)
 	sscanf(line, "%hu", _cluster.port);
 if (lineno == LOCK_FILE_LINE_SOCKET_DIR)
 {
-	int			len;
-
+	/* strip trailing newline and carriage return */
 	cluster->sockdir = pg_strdup(line);
-	/* strip off newline, handling Windows newlines as well */
-	len = strlen(cluster->sockdir);
-	while (len > 0 &&
-		   (cluster->sockdir[len - 1] == '\n' ||
-			cluster->sockdir[len - 1] == '\r'))
-		cluster->sockdir[--len] = '\0';
+	(void) pg_strip_crlf(cluster->sockdir);
 }
 			}
 			fclose(fp);
diff --git a/src/bin/psql/prompt.c b/src/bin/psql/prompt.c
index 59afbc793a..0fcb8c7783 100644
--- a/src/bin/psql/prompt.c
+++ b/src/bin/psql/prompt.c
@@ -22,6 +22,7 @@
 #include "prompt.h"
 #include "settings.h"
 
+#include "common/string.h"
 
 /*--
  * get_prompt
@@ -264,7 +265,6 @@ get_prompt(promptStatus_t status, ConditionalStack cstack)
 		FILE	   *fd;
 		char	   *file = pg_strdup(p + 1);
 		int			cmdend;
-		int			buflen;
 
 		cmdend = strcspn(file, "`");
 		file[cmdend] = '\0';
@@ -275,10 +275,10 @@ get_prompt(promptStatus_t status, ConditionalStack cstack)
 buf[0] = '\0';
 			pclose(fd);
 		}
-		buflen = strlen(buf);
-		while (buflen > 0 && (buf[buflen - 1] == '\n' ||
-			  buf[buflen - 1] == '\r'))
-			buf[--buflen] = '\0';
+
+		/* strip trailing newline and carriage