Hello PostgreSQL developers, 9.0beta5 seems to enable -Werror by default (which is a good thing, thanks!). FORTIFY_SOURCE catches a few places where the result of write() and fgets() is not checked, and thus the build fails with
gcc -g -O2 -g -Wall -O2 -fPIC -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing -fwrapv -g -Werror -I../../../../src/include -D_GNU_SOURCE -I/usr/include/libxml2 -I/usr/include/tcl8.5 -c -o elog.o elog.c cc1: warnings being treated as errors elog.c: In function 'write_console': elog.c:1698: error: ignoring return value of 'write', declared with attribute warn_unused_result elog.c: In function 'write_pipe_chunks': elog.c:2390: error: ignoring return value of 'write', declared with attribute warn_unused_result elog.c:2399: error: ignoring return value of 'write', declared with attribute warn_unused_result make[4]: *** [elog.o] Error 1 [...] gcc -g -O2 -g -Wall -O2 -fPIC -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing -fwrapv -g -Werror -pthread -D_REENTRANT -D_THREAD_SAFE -D_POSIX_PTHREAD_SEMANTICS -fpic -DFRONTEND -DUNSAFE_STAT_OK -I. -I../../../src/include -D_GNU_SOURCE -I/usr/include/libxml2 -I/usr/include/tcl8.5 -I../../../src/port -I../../../src/port -DSO_MAJOR_VERSION=5 -c -o fe-auth.o fe-auth.c gcc -g -O2 -g -Wall -O2 -fPIC -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing -fwrapv -g -Werror -pthread -D_REENTRANT -D_THREAD_SAFE -D_POSIX_PTHREAD_SEMANTICS -fpic -DFRONTEND -DUNSAFE_STAT_OK -I. -I../../../src/include -D_GNU_SOURCE -I/usr/include/libxml2 -I/usr/include/tcl8.5 -I../../../src/port -I../../../src/port -DSO_MAJOR_VERSION=5 -c -o fe-connect.o fe-connect.c cc1: warnings being treated as errors fe-connect.c: In function ‘PasswordFromFile’: fe-connect.c:4403: error: ignoring return value of ‘fgets’, declared with attribute warn_unused_result etc. I attach a patch (against git head) to check the results of those. For src/bin/psql/common.c this is really just an "ignore the result", but in src/bin/psql/prompt.c it actually fixes a potential crash. Thank you for considering! Martin -- Martin Pitt | http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From 398fc97d911cfedea5204eba584ce1e589d2b2b0 Mon Sep 17 00:00:00 2001
From: Martin Pitt <mp...@debian.org>
Date: Fri, 30 Apr 2010 12:43:13 +0200
Subject: [PATCH] Check results from fgets() and write() calls.
While most of them are harmless, they lead to a build failure with -Werror
(which is enabled by default in alpha releases). The one in fe-connect.c fixes
a real potential crasher, though.
---
src/backend/utils/error/elog.c | 12 +++++++++---
src/bin/psql/common.c | 6 +++++-
src/bin/psql/prompt.c | 3 ++-
src/interfaces/libpq/fe-connect.c | 3 ++-
4 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 1b1e3e9..59c85f8 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -1653,6 +1653,8 @@ write_eventlog(int level, const char *line, int len)
static void
write_console(const char *line, int len)
{
+ int res;
+
#ifdef WIN32
/*
@@ -1695,7 +1697,8 @@ write_console(const char *line, int len)
*/
#endif
- write(fileno(stderr), line, len);
+ res = write(fileno(stderr), line, len);
+ Assert(res == len);
}
/*
@@ -2375,6 +2378,7 @@ write_pipe_chunks(char *data, int len, int dest)
PipeProtoChunk p;
int fd = fileno(stderr);
+ int res;
Assert(len > 0);
@@ -2387,7 +2391,8 @@ write_pipe_chunks(char *data, int len, int dest)
p.proto.is_last = (dest == LOG_DESTINATION_CSVLOG ? 'F' : 'f');
p.proto.len = PIPE_MAX_PAYLOAD;
memcpy(p.proto.data, data, PIPE_MAX_PAYLOAD);
- write(fd, &p, PIPE_HEADER_SIZE + PIPE_MAX_PAYLOAD);
+ res = write(fd, &p, PIPE_HEADER_SIZE + PIPE_MAX_PAYLOAD);
+ Assert(res == PIPE_HEADER_SIZE + PIPE_MAX_PAYLOAD);
data += PIPE_MAX_PAYLOAD;
len -= PIPE_MAX_PAYLOAD;
}
@@ -2396,7 +2401,8 @@ write_pipe_chunks(char *data, int len, int dest)
p.proto.is_last = (dest == LOG_DESTINATION_CSVLOG ? 'T' : 't');
p.proto.len = len;
memcpy(p.proto.data, data, len);
- write(fd, &p, PIPE_HEADER_SIZE + len);
+ res = write(fd, &p, PIPE_HEADER_SIZE + len);
+ Assert(res == PIPE_HEADER_SIZE + len);
}
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index f605c97..695817e 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -219,7 +219,9 @@ static PGcancel *volatile cancelConn = NULL;
static CRITICAL_SECTION cancelConnLock;
#endif
-#define write_stderr(str) write(fileno(stderr), str, strlen(str))
+ /* ignore result of write(); it can't ENOSPC on stderr, and if it fails
+ * (EBADF, EPIPE, etc.) there is not much we can do anyway */
+#define write_stderr(str) if (write(fileno(stderr), str, strlen(str))) {}
#ifndef WIN32
@@ -244,7 +246,9 @@ handle_sigint(SIGNAL_ARGS)
if (cancelConn != NULL)
{
if (PQcancel(cancelConn, errbuf, sizeof(errbuf)))
+ {
write_stderr("Cancel request sent\n");
+ }
else
{
write_stderr("Could not send cancel request: ");
diff --git a/src/bin/psql/prompt.c b/src/bin/psql/prompt.c
index 4c346ce..6cd5b3e 100644
--- a/src/bin/psql/prompt.c
+++ b/src/bin/psql/prompt.c
@@ -252,7 +252,8 @@ get_prompt(promptStatus_t status)
fd = popen(file, "r");
if (fd)
{
- fgets(buf, sizeof(buf), fd);
+ if (fgets(buf, sizeof(buf), fd) == NULL)
+ buf[0] = '\0';
pclose(fd);
}
if (strlen(buf) > 0 && buf[strlen(buf) - 1] == '\n')
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 127d634..06f2681 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -4400,7 +4400,8 @@ PasswordFromFile(char *hostname, char *port, char *dbname, char *username)
*ret;
int len;
- fgets(buf, sizeof(buf), fp);
+ if (fgets(buf, sizeof(buf), fp) == NULL)
+ break;
len = strlen(buf);
if (len == 0)
--
1.5.6.5
signature.asc
Description: Digital signature
