public inbox for [email protected]  
help / color / mirror / Atom feed
From: Stuart Gathman <[email protected]>
To: [email protected]
Subject: Null this in pgConn::GetStatus
Date: Fri, 8 Jul 2016 10:23:19 -0400
Message-ID: <[email protected]> (raw)
List-Unsubscribe:  <mailto:[email protected]?body=unsub%20pgadmin-hackers>

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.





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


Attachments:

  [text/x-patch] pgadmin3-nullthis.patch (1.1K, 2-pgadmin3-nullthis.patch)
  download | inline diff:
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;





view thread (4+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected]
  Subject: Re: Null this in pgConn::GetStatus
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox