exit() calls in libraries

Started by Peter Eisentrautabout 14 years ago3 messages
#1Peter Eisentraut
peter_e@gmx.net

Debian's package policy and quality check tool lintian reports the
following (among other things) on the postgresql-9.1 (and earlier)
packages:

X: libpq5: shlib-calls-exit usr/lib/libpq.so.5.4
X: libecpg6: shlib-calls-exit usr/lib/libecpg.so.6.3

which is explained as

I: shlib-calls-exit
N:
N: The listed shared library calls the C library exit() or _exit()
N: functions.
N:
N: In the case of an error, the library should instead return an
N: appropriate error code to the calling program which can then determine
N: how to handle the error, including performing any required clean-up.
[snip]

The report on libecpg is actually a false positive, because the exit()
call comes from get_progname() in path.c, which is not called from
functions in libecpg.

The cases in libpq are

* various places in fe-print.c calling exit(1) when malloc fails,
presumably having run out of memory, and
* in libpq-int.h the macro PGTHREAD_ERROR, which is called in
several places in fe-connect.c and fe-secure.c.

Are these appropriate behaviors? The fe-print.c stuff probably isn't
used much anymore. But the threading stuff is, and it encroaches on the
exit status space of the libpq-using program. And does it even make
sense to call exit() if the thread locking is busted? Maybe abort()
would work better?

#2Alvaro Herrera
alvherre@commandprompt.com
In reply to: Peter Eisentraut (#1)
Re: exit() calls in libraries

Excerpts from Peter Eisentraut's message of lun dic 05 14:27:41 -0300 2011:

The cases in libpq are

* various places in fe-print.c calling exit(1) when malloc fails,
presumably having run out of memory, and
* in libpq-int.h the macro PGTHREAD_ERROR, which is called in
several places in fe-connect.c and fe-secure.c.

Are these appropriate behaviors? The fe-print.c stuff probably isn't
used much anymore. But the threading stuff is, and it encroaches on the
exit status space of the libpq-using program. And does it even make
sense to call exit() if the thread locking is busted? Maybe abort()
would work better?

Having had to battle some exit() calls in the PHP interpreter back when
I was working in PL/php, I agree that they shouldn't be there -- abort()
seems more appropriate if the system is truly busted. As for the
fr-print.c code, I'm not really sure why don't we just remove it.

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

#3Peter Eisentraut
peter_e@gmx.net
In reply to: Alvaro Herrera (#2)
1 attachment(s)
Re: exit() calls in libraries

On mån, 2011-12-05 at 15:04 -0300, Alvaro Herrera wrote:

Having had to battle some exit() calls in the PHP interpreter back
when
I was working in PL/php, I agree that they shouldn't be there --
abort()
seems more appropriate if the system is truly busted. As for the
fr-print.c code, I'm not really sure why don't we just remove it.

This is the patch that would get rid of all exit() calls in the
libraries.

Attachments:

pg-shlib-exit.patchtext/x-patch; charset=UTF-8; name=pg-shlib-exit.patchDownload
diff --git i/src/interfaces/libpq/fe-print.c w/src/interfaces/libpq/fe-print.c
index 5fa3be0..a6c2959 100644
--- i/src/interfaces/libpq/fe-print.c
+++ w/src/interfaces/libpq/fe-print.c
@@ -112,17 +112,17 @@ PQprint(FILE *fout, const PGresult *res, const PQprintOpt *po)
 		if (!(fieldNames = (const char **) calloc(nFields, sizeof(char *))))
 		{
 			fprintf(stderr, libpq_gettext("out of memory\n"));
-			exit(1);
+			abort();
 		}
 		if (!(fieldNotNum = (unsigned char *) calloc(nFields, 1)))
 		{
 			fprintf(stderr, libpq_gettext("out of memory\n"));
-			exit(1);
+			abort();
 		}
 		if (!(fieldMax = (int *) calloc(nFields, sizeof(int))))
 		{
 			fprintf(stderr, libpq_gettext("out of memory\n"));
-			exit(1);
+			abort();
 		}
 		for (numFieldName = 0;
 			 po->fieldName && po->fieldName[numFieldName];
@@ -203,7 +203,7 @@ PQprint(FILE *fout, const PGresult *res, const PQprintOpt *po)
 			if (!(fields = (char **) calloc(nFields * (nTups + 1), sizeof(char *))))
 			{
 				fprintf(stderr, libpq_gettext("out of memory\n"));
-				exit(1);
+				abort();
 			}
 		}
 		else if (po->header && !po->html3)
@@ -391,7 +391,7 @@ do_field(const PQprintOpt *po, const PGresult *res,
 			if (!(fields[i * nFields + j] = (char *) malloc(plen + 1)))
 			{
 				fprintf(stderr, libpq_gettext("out of memory\n"));
-				exit(1);
+				abort();
 			}
 			strcpy(fields[i * nFields + j], pval);
 		}
@@ -462,7 +462,7 @@ do_header(FILE *fout, const PQprintOpt *po, const int nFields, int *fieldMax,
 		if (!border)
 		{
 			fprintf(stderr, libpq_gettext("out of memory\n"));
-			exit(1);
+			abort();
 		}
 		p = border;
 		if (po->standard)
@@ -605,7 +605,7 @@ PQdisplayTuples(const PGresult *res,
 		if (!fLength)
 		{
 			fprintf(stderr, libpq_gettext("out of memory\n"));
-			exit(1);
+			abort();
 		}
 
 		for (j = 0; j < nFields; j++)
@@ -704,7 +704,7 @@ PQprintTuples(const PGresult *res,
 			if (!tborder)
 			{
 				fprintf(stderr, libpq_gettext("out of memory\n"));
-				exit(1);
+				abort();
 			}
 			for (i = 0; i <= width; i++)
 				tborder[i] = '-';
diff --git i/src/interfaces/libpq/libpq-int.h w/src/interfaces/libpq/libpq-int.h
index 64dfcb2..a0d93e0 100644
--- i/src/interfaces/libpq/libpq-int.h
+++ w/src/interfaces/libpq/libpq-int.h
@@ -483,7 +483,7 @@ extern pgthreadlock_t pg_g_threadlock;
 #define PGTHREAD_ERROR(msg) \
 	do { \
 		fprintf(stderr, "%s\n", msg); \
-		exit(1); \
+		abort(); \
 	} while (0)
 
 
diff --git i/src/port/path.c w/src/port/path.c
index 9cb0b01..de345c2 100644
--- i/src/port/path.c
+++ w/src/port/path.c
@@ -445,7 +445,7 @@ get_progname(const char *argv0)
 	if (progname == NULL)
 	{
 		fprintf(stderr, "%s: out of memory\n", nodir_name);
-		exit(1);				/* This could exit the postmaster */
+		abort();				/* This could exit the postmaster */
 	}
 
 #if defined(__CYGWIN__) || defined(WIN32)