On 3 August 2011 12:19, Heikki Linnakangas
<heikki.linnakan...@enterprisedb.com> wrote:

> Right, but the purpose of that check is to defend from programmer error. If
> the programmer screws up and calls "PQresStatus(-1)", we want to give an
> error, not crash. If you assume that the programmer will only pass a valid
> enum constant as parameter, then you might as well remove the if-statement
> altogether. I don't think that would be an improvement.

Ahh. I failed to consider the intent of the code.

Attached patch has a better solution than casting though - we simply
use an enum literal. The fact that PGRES_EMPTY_QUERY is the first
value in the enum can be safely assumed to be stable, not least
because we've even already explicitly given it a corresponding value
of 0.

-- 
Peter Geoghegan       http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 605d242..b6e6363 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -2386,7 +2386,7 @@ PQresultStatus(const PGresult *res)
 char *
 PQresStatus(ExecStatusType status)
 {
-	if (status < 0 || status >= sizeof pgresStatus / sizeof pgresStatus[0])
+	if (status < PGRES_EMPTY_QUERY || status >= sizeof pgresStatus / sizeof pgresStatus[0])
 		return libpq_gettext("invalid ExecStatusType code");
 	return pgresStatus[status];
 }
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to