On Mon, Jun 08, 2009 at 10:10:09AM +0200, Mikael Lund Jepsen, ICCC wrote: > Bernhard Reutner-Fischer wrote: >> I suggest you base off the (completely untested) >> b32dd708df63d90f9d755b6c0de2588200e432ca >> Please also add testsuite entries. >> > Apart from the use of the GNU extension, this is very similar to what I > had, but I do have one comment though: > There is no check on the return value of snprintf, so if name is > (silently) truncated, two different names may trunctate to the same > filename - and the user will never know what hit him. Checking if > snprintf returned NAME_MAX or more and then setting ENAMETOOLONG should > do the trick... > > Even though the man page of shm_open does not mention it, I think it > would be nice if we set ENOMEM if malloc fails, since dynamic allocation > is special to our implementation.
I think that malloc would have set ENOMEM already so i will drop that bit. > > I've included a patch with my suggested changes and my testsuite entry. Will apply shortly. Thanks! > > TIA, > Mikael >>From 97c08e96db0deb037626cd90470e299707379849 Mon Sep 17 00:00:00 2001 >From: Mikael Lund Jepsen <[email protected]> >Date: Mon, 8 Jun 2009 10:02:48 +0200 >Subject: [PATCH] Set errno for shm_open and shm_unlink and add tests to >testsuite > >Untested > >Signed-off-by: Mikael Lund Jepsen <[email protected]> >--- > librt/shm.c | 9 ++++- > test/librt/Makefile | 6 +++ > test/librt/shmtest.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 116 insertions(+), 1 deletions(-) > create mode 100644 test/librt/Makefile > create mode 100644 test/librt/shmtest.c > >diff --git a/librt/shm.c b/librt/shm.c >index 637e945..fce83c6 100644 >--- a/librt/shm.c >+++ b/librt/shm.c >@@ -42,13 +42,20 @@ static char* get_shm_name(const char*name) > return NULL; > #else > path = malloc(NAME_MAX); >- if (path == NULL) >+ if (path == NULL) { >+ __set_errno(ENOMEM); > return NULL; >+ } > i = snprintf(path, NAME_MAX, _PATH_SHM "%s", name); > if (i < 0) { > free(path); > return NULL; > } >+ if (i >= NAME_MAX) { >+ free(path); >+ __set_errno(ENAMETOOLONG); >+ return NULL; >+ } > #endif > return path; > } >diff --git a/test/librt/Makefile b/test/librt/Makefile >new file mode 100644 >index 0000000..362d5d5 >--- /dev/null >+++ b/test/librt/Makefile >@@ -0,0 +1,6 @@ >+# uClibc shm tests >+# Licensed under the LGPL v2.1, see the file COPYING.LIB in this tarball. >+ >+include ../Test.mak >+ >+LDFLAGS_shmtest := -lrt >diff --git a/test/librt/shmtest.c b/test/librt/shmtest.c >new file mode 100644 >index 0000000..8fb166d >--- /dev/null >+++ b/test/librt/shmtest.c >@@ -0,0 +1,102 @@ >+/* Copyright (C) 2009 Mikael Lund Jepsen <[email protected]> >+ * >+ * Licensed under the LGPL v2.1, see the file COPYING.LIB in this tarball. >+ */ >+ >+#include <errno.h> >+#include <fcntl.h> >+#include <string.h> >+#include <stdio.h> >+#include <stdlib.h> >+#include <unistd.h> >+#include <sys/types.h> >+#include <sys/mman.h> >+#include <sys/stat.h> >+#include <sys/wait.h> >+ >+char shared_name[] = "/sharetest"; >+int test_data[11] = {0,1,2,3,4,5,6,7,8,9,10}; >+ >+int main(void) { >+ int pfds[2]; >+ pid_t pid; >+ int fd; >+ int test_data_fails = 0; >+ char *ptest_data; >+ unsigned int i; >+ char buf[30]; >+ int rv; >+ >+ pipe(pfds); >+ >+ switch(pid = fork()) { >+ case -1: >+ perror("fork"); >+ exit(1); // parent exits >+ >+ case 0: >+ // Child >+ >+ // wait for parent >+ read(pfds[0], buf, 5); >+ >+ fd = shm_open(shared_name, O_RDWR, DEFFILEMODE); >+ if (fd == -1) { >+ perror("CHILD - shm_open(existing):"); >+ exit(1); >+ } else { >+ ptest_data = mmap(0, sizeof(test_data), PROT_READ + >PROT_WRITE, MAP_SHARED, fd, 0); >+ if (ptest_data != MAP_FAILED) { >+ for (i=0; i < sizeof(test_data); i++) { >+ if (ptest_data[i] != test_data[i]) { >+ printf("%-40s: Offset %d, local >%d, shm %d", "Compare memory error", i, test_data[i], ptest_data[i]); >+ test_data_fails++; >+ } >+ } >+ if (test_data_fails == 0) >+ printf("%-40s: %s\n", "Compare memory", >"Success"); >+ >+ munmap(ptest_data, sizeof(test_data)); >+ } >+ } >+ exit(0); >+ >+ default: >+ // Parent >+ fd = shm_open(shared_name, O_RDWR+O_CREAT+O_EXCL, DEFFILEMODE ); >+ if (fd == -1) { >+ perror("PARENT - shm_open(create):"); >+ } else { >+ if ((ftruncate(fd, sizeof(test_data))) == -1) >+ { >+ printf("%-40s: %s", "ftruncate", >strerror(errno)); >+ shm_unlink(shared_name); >+ return 0; >+ } >+ >+ ptest_data = mmap(0, sizeof(test_data), PROT_READ + >PROT_WRITE, MAP_SHARED, fd, 0); >+ if (ptest_data == MAP_FAILED) >+ { >+ perror("PARENT - mmap:"); >+ if (shm_unlink(shared_name) == -1) { >+ perror("PARENT - shm_unlink:"); >+ } >+ return 0; >+ } >+ for (i=0; i <sizeof(test_data); i++) >+ ptest_data[i] = test_data[i]; >+ >+ // signal child >+ write(pfds[1], "rdy", 5); >+ // wait for child >+ wait(&rv); >+ >+ // Cleanup >+ munmap(ptest_data, sizeof(test_data)); >+ if (shm_unlink(shared_name) == -1) { >+ perror("PARENT - shm_unlink:"); >+ } >+ } >+ } >+ return 0; >+} >-- >1.5.4.3 > _______________________________________________ uClibc mailing list [email protected] http://lists.busybox.net/mailman/listinfo/uclibc
