[Openais] [PATCH] ipc: Fix error handling of mmap util functions.
Angus Salkeld
asalkeld at redhat.com
Mon Jun 28 16:43:54 PDT 2010
Steve, Not exactly the same as your latest patch ...
- use snprintf()
- use write to test that we can actually use all of the file.
Signed-off-by: Angus Salkeld <asalkeld at redhat.com>
---
exec/coroipcs.c | 74 +++++++++++++++++++++++++++++++++++-----------
lib/coroipcc.c | 88 ++++++++++++++++++++++++++++++++++++++++---------------
2 files changed, 120 insertions(+), 42 deletions(-)
diff --git a/exec/coroipcs.c b/exec/coroipcs.c
index a2deb8a..1a084d5 100644
--- a/exec/coroipcs.c
+++ b/exec/coroipcs.c
@@ -254,10 +254,13 @@ memory_map (
size_t bytes,
void **buf)
{
- int fd;
+ int32_t fd;
void *addr_orig;
void *addr;
- int res;
+ int32_t res;
+ char buffer[128];
+ int32_t i;
+ int32_t written;
fd = open (path, O_RDWR, 0600);
@@ -269,24 +272,33 @@ memory_map (
res = ftruncate (fd, bytes);
if (res == -1) {
- close (fd);
- return (-1);
+ goto error_close_unlink;
+ }
+ memset (buffer, 0, sizeof (buffer));
+ for (i = 0; i < (bytes / 64); i++) {
+retry_write:
+ written = write (fd, buffer, 64);
+ if (written == -1 && errno == EINTR) {
+ goto retry_write;
+ }
+ if (written != 64) {
+ goto error_close_unlink;
+ }
}
addr_orig = mmap (NULL, bytes, PROT_NONE,
MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
if (addr_orig == MAP_FAILED) {
- close (fd);
- return (-1);
+ goto error_close_unlink;
}
addr = mmap (addr_orig, bytes, PROT_READ | PROT_WRITE,
MAP_FIXED | MAP_SHARED, fd, 0);
if (addr != addr_orig) {
- close (fd);
- return (-1);
+ munmap(addr_orig, bytes);
+ goto error_close_unlink;
}
#ifdef COROSYNC_BSD
madvise(addr, bytes, MADV_NOSYNC);
@@ -298,6 +310,11 @@ memory_map (
}
*buf = addr_orig;
return (0);
+
+error_close_unlink:
+ close (fd);
+ unlink(path);
+ return -1;
}
static int
@@ -306,10 +323,13 @@ circular_memory_map (
size_t bytes,
void **buf)
{
- int fd;
+ int32_t fd;
void *addr_orig;
void *addr;
- int res;
+ int32_t res;
+ char buffer[128];
+ int32_t i;
+ int32_t written;
fd = open (path, O_RDWR, 0600);
@@ -320,24 +340,34 @@ circular_memory_map (
}
res = ftruncate (fd, bytes);
if (res == -1) {
- close (fd);
- return (-1);
+ goto error_close_unlink;
+ }
+ memset (buffer, 0, sizeof (buffer));
+ for (i = 0; i < (bytes / 64); i++) {
+retry_write:
+ written = write (fd, buffer, 64);
+ if (written == -1 && errno == EINTR) {
+ goto retry_write;
+ }
+ if (written != 64) {
+ goto error_close_unlink;
+ }
}
addr_orig = mmap (NULL, bytes << 1, PROT_NONE,
MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
if (addr_orig == MAP_FAILED) {
- close (fd);
- return (-1);
+ munmap(addr_orig, bytes);
+ goto error_close_unlink;
}
addr = mmap (addr_orig, bytes, PROT_READ | PROT_WRITE,
MAP_FIXED | MAP_SHARED, fd, 0);
if (addr != addr_orig) {
- close (fd);
- return (-1);
+ munmap(addr_orig, bytes);
+ goto error_close_unlink;
}
#ifdef COROSYNC_BSD
madvise(addr_orig, bytes, MADV_NOSYNC);
@@ -347,8 +377,9 @@ circular_memory_map (
bytes, PROT_READ | PROT_WRITE,
MAP_FIXED | MAP_SHARED, fd, 0);
if (addr == MAP_FAILED) {
- close (fd);
- return (-1);
+ munmap(addr_orig, bytes);
+ munmap(addr, bytes);
+ goto error_close_unlink;
}
#ifdef COROSYNC_BSD
madvise(((char *)addr_orig) + bytes, bytes, MADV_NOSYNC);
@@ -356,10 +387,17 @@ circular_memory_map (
res = close (fd);
if (res) {
+ munmap(addr_orig, bytes);
+ munmap(addr, bytes);
return (-1);
}
*buf = addr_orig;
return (0);
+
+error_close_unlink:
+ close (fd);
+ unlink(path);
+ return (-1);
}
static inline int
diff --git a/lib/coroipcc.c b/lib/coroipcc.c
index 0d2d13b..944a85c 100644
--- a/lib/coroipcc.c
+++ b/lib/coroipcc.c
@@ -40,6 +40,7 @@
#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
+#include <limits.h>
#include <errno.h>
#include <string.h>
#include <fcntl.h>
@@ -283,16 +284,19 @@ union semun {
static int
circular_memory_map (char *path, const char *file, void **buf, size_t bytes)
{
- int fd;
+ int32_t fd;
void *addr_orig;
void *addr;
- int res;
+ int32_t res;
+ char buffer[128];
+ int32_t i;
+ int32_t written;
- sprintf (path, "/dev/shm/%s", file);
+ snprintf (path, PATH_MAX, "/dev/shm/%s", file);
fd = mkstemp (path);
if (fd == -1) {
- sprintf (path, LOCALSTATEDIR "/run/%s", file);
+ snprintf (path, PATH_MAX, LOCALSTATEDIR "/run/%s", file);
fd = mkstemp (path);
if (fd == -1) {
return (-1);
@@ -301,24 +305,33 @@ circular_memory_map (char *path, const char *file, void **buf, size_t bytes)
res = ftruncate (fd, bytes);
if (res == -1) {
- close (fd);
- return (-1);
+ goto error_close_unlink;
+ }
+ memset (buffer, 0, sizeof (buffer));
+ for (i = 0; i < (bytes / 64); i++) {
+retry_write:
+ written = write (fd, buffer, 64);
+ if (written == -1 && errno == EINTR) {
+ goto retry_write;
+ }
+ if (written != 64) {
+ goto error_close_unlink;
+ }
}
addr_orig = mmap (NULL, bytes << 1, PROT_NONE,
MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
if (addr_orig == MAP_FAILED) {
- close (fd);
- return (-1);
+ goto error_close_unlink;
}
addr = mmap (addr_orig, bytes, PROT_READ | PROT_WRITE,
MAP_FIXED | MAP_SHARED, fd, 0);
if (addr != addr_orig) {
- close (fd);
- return (-1);
+ munmap(addr_orig, bytes);
+ goto error_close_unlink;
}
#ifdef COROSYNC_BSD
madvise(addr_orig, bytes, MADV_NOSYNC);
@@ -328,8 +341,9 @@ circular_memory_map (char *path, const char *file, void **buf, size_t bytes)
bytes, PROT_READ | PROT_WRITE,
MAP_FIXED | MAP_SHARED, fd, 0);
if (addr == MAP_FAILED) {
- close (fd);
- return (-1);
+ munmap(addr_orig, bytes);
+ munmap(addr, bytes);
+ goto error_close_unlink;
}
#ifdef COROSYNC_BSD
madvise(((char *)addr_orig) + bytes, bytes, MADV_NOSYNC);
@@ -337,10 +351,17 @@ circular_memory_map (char *path, const char *file, void **buf, size_t bytes)
res = close (fd);
if (res) {
+ munmap(addr_orig, bytes);
+ munmap(addr, bytes);
return (-1);
}
*buf = addr_orig;
return (0);
+
+error_close_unlink:
+ close (fd);
+ unlink(path);
+ return (-1);
}
static void
@@ -362,19 +383,23 @@ void ipc_hdb_destructor (void *context ) {
memory_unmap (ipc_instance->response_buffer, ipc_instance->response_size);
memory_unmap (ipc_instance->dispatch_buffer, (ipc_instance->dispatch_size) << 1);
}
+
static int
memory_map (char *path, const char *file, void **buf, size_t bytes)
{
- int fd;
+ int32_t fd;
void *addr_orig;
void *addr;
- int res;
+ int32_t res;
+ char buffer[128];
+ int32_t i;
+ int32_t written;
- sprintf (path, "/dev/shm/%s", file);
+ snprintf (path, PATH_MAX, "/dev/shm/%s", file);
fd = mkstemp (path);
if (fd == -1) {
- sprintf (path, LOCALSTATEDIR "/run/%s", file);
+ snprintf (path, PATH_MAX, LOCALSTATEDIR "/run/%s", file);
fd = mkstemp (path);
if (fd == -1) {
return (-1);
@@ -383,24 +408,33 @@ memory_map (char *path, const char *file, void **buf, size_t bytes)
res = ftruncate (fd, bytes);
if (res == -1) {
- close (fd);
- return (-1);
+ goto error_close_unlink;
+ }
+ memset (buffer, 0, sizeof (buffer));
+ for (i = 0; i < (bytes / 64); i++) {
+retry_write:
+ written = write (fd, buffer, 64);
+ if (written == -1 && errno == EINTR) {
+ goto retry_write;
+ }
+ if (written != 64) {
+ goto error_close_unlink;
+ }
}
addr_orig = mmap (NULL, bytes, PROT_NONE,
MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
if (addr_orig == MAP_FAILED) {
- close (fd);
- return (-1);
+ goto error_close_unlink;
}
addr = mmap (addr_orig, bytes, PROT_READ | PROT_WRITE,
MAP_FIXED | MAP_SHARED, fd, 0);
if (addr != addr_orig) {
- close (fd);
- return (-1);
+ munmap(addr_orig, bytes);
+ goto error_close_unlink;
}
#ifdef COROSYNC_BSD
madvise(addr_orig, bytes, MADV_NOSYNC);
@@ -411,7 +445,13 @@ memory_map (char *path, const char *file, void **buf, size_t bytes)
return (-1);
}
*buf = addr_orig;
- return (0);
+
+ return 0;
+
+error_close_unlink:
+ close (fd);
+ unlink(path);
+ return -1;
}
static cs_error_t
@@ -479,7 +519,7 @@ ipc_sem_wait (
#else
struct timespec timeout;
struct pollfd pfd;
- sem_t *sem;
+ sem_t *sem = NULL;
#endif
int res;
--
1.7.1
More information about the Openais
mailing list