public inbox for [email protected]
help / color / mirror / Atom feedNull this in pgConn::GetStatus
4+ messages / 1 participants
[nested] [flat]
* Null this in pgConn::GetStatus
@ 2016-07-04 02:44 Stuart Gathman <[email protected]>
0 siblings, 1 reply; 4+ messages in thread
From: Stuart Gathman @ 2016-07-04 02:44 UTC (permalink / raw)
To: pgadmin-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;
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Null this in pgConn::GetStatus
@ 2016-07-04 03:19 Stuart Gathman <[email protected]>
parent: Stuart Gathman <[email protected]>
0 siblings, 0 replies; 4+ messages in thread
From: Stuart Gathman @ 2016-07-04 03:19 UTC (permalink / raw)
To: pgadmin-hackers
On 07/03/2016 10:44 PM, Stuart Gathman wrote:
> 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:
Hmmm. sqlTable::Paste is also broken - and it is not as trivial to
convert to a static function.
--
Sent via pgadmin-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers
^ permalink raw reply [nested|flat] 4+ messages in thread
* Null this in pgConn::GetStatus
@ 2016-07-08 14:23 Stuart Gathman <[email protected]>
0 siblings, 0 replies; 4+ messages in thread
From: Stuart Gathman @ 2016-07-08 14:23 UTC (permalink / raw)
To: pgadmin-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;
^ permalink raw reply [nested|flat] 4+ messages in thread
* [pgadmin3][Patch] Null this in pgConn::GetStatus
@ 2016-07-08 14:45 Stuart Gathman <[email protected]>
0 siblings, 0 replies; 4+ messages in thread
From: Stuart Gathman @ 2016-07-08 14:45 UTC (permalink / raw)
To: pgadmin-hackers
Saw the convention to tag for pgadmin3 and patch, so posting a 3rd time.
Sorry - new to pgadmin.
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;
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2016-07-08 14:45 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2016-07-04 02:44 Null this in pgConn::GetStatus Stuart Gathman <[email protected]>
2016-07-04 03:19 ` Stuart Gathman <[email protected]>
2016-07-08 14:23 Null this in pgConn::GetStatus Stuart Gathman <[email protected]>
2016-07-08 14:45 [pgadmin3][Patch] Null this in pgConn::GetStatus Stuart Gathman <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox