From: Stuart Gathman <stuart@gathman.org>
Subject: Null this in pgConn::GetStatus
To: pgadmin-hackers@postgresql.org
Organization: Gathman Systems
Jabber-Id: stuart@gathman.org
Message-ID: <02898a62-c592-6eb9-1ff9-f76ac34eeb57@gathman.org>
Date: Sun, 3 Jul 2016 22:44:07 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.1.1
MIME-Version: 1.0
Content-Type: multipart/mixed;
 boundary="------------675C9EA27BED9113EFB693D7"
Precedence: bulk
Sender: pgadmin-hackers-owner@postgresql.org

This is a multi-part message in MIME format.
--------------675C9EA27BED9113EFB693D7
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit

pgadmin3-1.22.1

When compiled with gcc6, the evil practice of "this == 0" fails
horribly.  The most obvious effect is crashing during startup.  I've
attached a simple patch.  The Fedora bugzilla is here:

https://bugzilla.redhat.com/show_bug.cgi?id=1335043

A detailed explanation of why this == 0 is evil:
http://www.viva64.com/en/b/0226/


The patch moves the guts of pgConn::GetStatus() to a static function,
where comparing the ptr arg to 0 is well defined, and makes the
GetStatus() member function call the static function inline.  It is
still technically undefined when calling the GetStatus member function
on a null ptr - but for now the compiler is not eliminating the test.
It is also a step in right direction, which would be to use the static
function at calling sites where the ptr could be null.


--------------675C9EA27BED9113EFB693D7
Content-Type: text/x-patch;
 name="pgadmin3-nullthis.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="pgadmin3-nullthis.patch"

diff -up ./pgadmin/db/pgConn.cpp.nullthis ./pgadmin/db/pgConn.cpp
--- ./pgadmin/db/pgConn.cpp.nullthis	2016-07-01 13:18:44.649386521 -0400
+++ ./pgadmin/db/pgConn.cpp	2016-07-01 13:25:40.815813995 -0400
@@ -1003,15 +1003,15 @@ bool pgConn::IsAlive()
 }
 
 
-int pgConn::GetStatus() const
+int pgConn::GetStatus(const pgConn *p)
 {
-	if (!this)
+	if (!p)
 		return PGCONN_BAD;
 
-	if (conn)
-		((pgConn *)this)->connStatus = PQstatus(conn);
+	if (p->conn)	// FIXME: casting away const !
+		((pgConn *)p)->connStatus = PQstatus(p->conn);
 
-	return connStatus;
+	return p->connStatus;
 }
 
 
diff -up ./pgadmin/include/db/pgConn.h.nullthis ./pgadmin/include/db/pgConn.h
--- ./pgadmin/include/db/pgConn.h.nullthis	2016-07-01 13:18:56.643512630 -0400
+++ ./pgadmin/include/db/pgConn.h	2016-07-01 13:24:07.339776340 -0400
@@ -215,7 +215,11 @@ public:
 	{
 		return PQbackendPID(conn);
 	}
-	int GetStatus() const;
+	static int GetStatus(const pgConn *);
+	int GetStatus() const
+	{
+		return GetStatus(this);
+	}
 	int GetLastResultStatus() const
 	{
 		return lastResultStatus;


--------------675C9EA27BED9113EFB693D7
Content-Type: text/plain
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0


-- 
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

--------------675C9EA27BED9113EFB693D7--