On Mon, Jun 28, 2010 at 04:58:42PM -0700, Steven Dake wrote: > On 06/28/2010 04:43 PM, Angus Salkeld wrote: > > 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<[email protected]> > > --- > > 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; > > > > > the fd should be closed when the mmap operation is done. This will > release the file descriptor from the process descriptor in the kernel > once munmap is executed. Otherwise this leaks file descriptors. > > A partial map will result in leakage ie: > sys_res = memory_map ( > response_map_path, > "response_buffer-XXXXXX", > (void *)&ipc_instance->response_buffer, > response_size); > if (sys_res == -1) { > res = CS_ERR_LIBRARY; > goto error_response_buffer; > } > > sys_res = circular_memory_map ( > dispatch_map_path, > "dispatch_buffer-XXXXXX", > (void *)&ipc_instance->dispatch_buffer, > dispatch_size); > if (sys_res == -1) { > res = CS_ERR_LIBRARY; > goto error_dispatch_buffer; > } > > if the dispatch fails, the response buffer will be leaked.
I am not getting you, all 4 functions have a close() on successful and failure cases. > > Regards > -steve > _______________________________________________ > Openais mailing list > [email protected] > https://lists.linux-foundation.org/mailman/listinfo/openais _______________________________________________ Openais mailing list [email protected] https://lists.linux-foundation.org/mailman/listinfo/openais
