On Mon, Jul 13, 2020 at 03:59:56PM +0900, Masahiko Sawada wrote:
> On Mon, 13 Jul 2020 at 15:34, Daniel Gustafsson <dan...@yesql.se> wrote:
>> Yeah, I agree with that, seems like the call should've been PQgetisnull(res, 
>> i, 1);
>> to match the loop.
> 
> +1

Good catch, Justin.  There is a second thing here.  The second column
matches with the file size, so if its value is NULL then atol() would
just crash first.  I think that it would be more simple to first check
if the file size is NULL (isdir and link_target would be also NULL,
but just checking for the file size is fine), and then assign the
result values, like in the attached.  Any thoughts?
--
Michael
diff --git a/src/bin/pg_rewind/libpq_fetch.c b/src/bin/pg_rewind/libpq_fetch.c
index 1dbbceab0b..c44648f823 100644
--- a/src/bin/pg_rewind/libpq_fetch.c
+++ b/src/bin/pg_rewind/libpq_fetch.c
@@ -214,13 +214,13 @@ libpqProcessFileList(void)
 	/* Read result to local variables */
 	for (i = 0; i < PQntuples(res); i++)
 	{
-		char	   *path = PQgetvalue(res, i, 0);
-		int64		filesize = atol(PQgetvalue(res, i, 1));
-		bool		isdir = (strcmp(PQgetvalue(res, i, 2), "t") == 0);
-		char	   *link_target = PQgetvalue(res, i, 3);
+		char	   *path;
+		int64		filesize;
+		bool		isdir;
+		char	   *link_target;
 		file_type_t type;
 
-		if (PQgetisnull(res, 0, 1))
+		if (PQgetisnull(res, i, 1))
 		{
 			/*
 			 * The file was removed from the server while the query was
@@ -229,6 +229,11 @@ libpqProcessFileList(void)
 			continue;
 		}
 
+		path = PQgetvalue(res, i, 0);
+		filesize = atol(PQgetvalue(res, i, 1));
+		isdir = (strcmp(PQgetvalue(res, i, 2), "t") == 0);
+		link_target = PQgetvalue(res, i, 3);
+
 		if (link_target[0])
 			type = FILE_TYPE_SYMLINK;
 		else if (isdir)

Attachment: signature.asc
Description: PGP signature

Reply via email to