Date: Thu, 31 Jan 2013 16:39:50 +0000 (UTC) From: Pietro Cerutti <gahr@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r246148 - in head: lib/libc/stdio tools/regression/lib/libc/stdio Message-ID: <201301311639.r0VGdo0Q044244@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: gahr (ports committer) Date: Thu Jan 31 16:39:50 2013 New Revision: 246148 URL: http://svnweb.freebsd.org/changeset/base/246148 Log: - Remove underscores from the internal structure name, as it doesn't collide with the user's namespace. - Correct size and position variables type from long to size_t. - Do not set errno to ENOMEM on malloc failure, as malloc already does so. - Implement the concept of "buffer data length", which mandates what SEEK_END refers to and the allowed extent for a read. - Use NULL as read-callback if the buffer is opened in write-only mode. Conversely, use NULL as write-callback when opened in read-only mode. - Implement the handling of the ``b'' character in the mode argument. A binary buffer differs from a text buffer (default mode if ``b'' is omitted) in that NULL bytes are never appended to writes and that the "buffer data length" equals to the size of the buffer. - Remove shall from the man page. Use indicative instead. Also, specify that the ``b'' flag does not conform with POSIX but is supported by glibc. - Update the regression test so that the ``b'' functionality and the "buffer data length" concepts are tested. - Minor style(9) corrections. Suggested by: jilles Reviewed by: cognet Approved by: cognet Modified: head/lib/libc/stdio/fmemopen.c head/lib/libc/stdio/fopen.3 head/tools/regression/lib/libc/stdio/test-fmemopen.c Modified: head/lib/libc/stdio/fmemopen.c ============================================================================== --- head/lib/libc/stdio/fmemopen.c Thu Jan 31 16:04:40 2013 (r246147) +++ head/lib/libc/stdio/fmemopen.c Thu Jan 31 16:39:50 2013 (r246148) @@ -26,17 +26,21 @@ SUCH DAMAGE. #include <sys/cdefs.h> __FBSDID("$FreeBSD$"); +#include <fcntl.h> #include <stdio.h> #include <stdlib.h> #include <string.h> #include <errno.h> +#include "local.h" -struct __fmemopen_cookie +struct fmemopen_cookie { - char *buf; /* pointer to the memory region */ - char own; /* did we allocate the buffer ourselves? */ - long len; /* buffer length in bytes */ - long off; /* current offset into the buffer */ + char *buf; /* pointer to the memory region */ + char own; /* did we allocate the buffer ourselves? */ + char bin; /* is this a binary buffer? */ + size_t size; /* buffer length in bytes */ + size_t len; /* data length in bytes */ + size_t off; /* current offset into the buffer */ }; static int fmemopen_read (void *cookie, char *buf, int nbytes); @@ -47,33 +51,95 @@ static int fmemopen_close (void *cookie) FILE * fmemopen (void * __restrict buf, size_t size, const char * __restrict mode) { - /* allocate cookie */ - struct __fmemopen_cookie *ck = malloc (sizeof (struct __fmemopen_cookie)); + struct fmemopen_cookie *ck; + FILE *f; + int flags, rc; + + /* + * Retrieve the flags as used by open(2) from the mode argument, and + * validate them. + * */ + rc = __sflags (mode, &flags); + if (rc == 0) { + errno = EINVAL; + return (NULL); + } + + /* + * There's no point in requiring an automatically allocated buffer + * in write-only mode. + */ + if (!(flags & O_RDWR) && buf == NULL) { + errno = EINVAL; + return (NULL); + } + + /* Allocate a cookie. */ + ck = malloc (sizeof (struct fmemopen_cookie)); if (ck == NULL) { - errno = ENOMEM; return (NULL); } - ck->off = 0; - ck->len = size; + ck->off = 0; + ck->size = size; - /* do we have to allocate the buffer ourselves? */ + /* Check whether we have to allocate the buffer ourselves. */ ck->own = ((ck->buf = buf) == NULL); if (ck->own) { ck->buf = malloc (size); if (ck->buf == NULL) { free (ck); - errno = ENOMEM; return (NULL); } + } + + /* + * POSIX distinguishes between w+ and r+, in that w+ is supposed to + * truncate the buffer. + */ + if (ck->own || mode[0] == 'w') { ck->buf[0] = '\0'; } - if (mode[0] == 'a') - ck->off = strnlen(ck->buf, ck->len); + /* Check for binary mode. */ + ck->bin = strchr(mode, 'b') != NULL; - /* actuall wrapper */ - FILE *f = funopen ((void *)ck, fmemopen_read, fmemopen_write, + /* + * The size of the current buffer contents is set depending on the + * mode: + * + * for append (text-mode), the position of the first NULL byte, or the + * size of the buffer if none is found + * + * for append (binary-mode), the size of the buffer + * + * for read, the size of the buffer + * + * for write, 0 + */ + switch (mode[0]) { + case 'a': + if (ck->bin) { + /* + * This isn't useful, since the buffer isn't + * allowed to grow. + */ + ck->off = ck->len = size; + } else + ck->off = ck->len = strnlen(ck->buf, ck->size); + break; + case 'r': + ck->len = size; + break; + case 'w': + ck->len = 0; + break; + } + + /* Actuall wrapper. */ + f = funopen ((void *)ck, + flags & O_WRONLY ? NULL : fmemopen_read, + flags & O_RDONLY ? NULL : fmemopen_write, fmemopen_seek, fmemopen_close); if (f == NULL) { @@ -83,8 +149,10 @@ fmemopen (void * __restrict buf, size_t return (NULL); } - /* turn off buffering, so a write past the end of the buffer - * correctly returns a short object count */ + /* + * Turn off buffering, so a write past the end of the buffer + * correctly returns a short object count. + */ setvbuf (f, (char *) NULL, _IONBF, 0); return (f); @@ -93,7 +161,7 @@ fmemopen (void * __restrict buf, size_t static int fmemopen_read (void *cookie, char *buf, int nbytes) { - struct __fmemopen_cookie *ck = cookie; + struct fmemopen_cookie *ck = cookie; if (nbytes > ck->len - ck->off) nbytes = ck->len - ck->off; @@ -111,10 +179,10 @@ fmemopen_read (void *cookie, char *buf, static int fmemopen_write (void *cookie, const char *buf, int nbytes) { - struct __fmemopen_cookie *ck = cookie; + struct fmemopen_cookie *ck = cookie; - if (nbytes > ck->len - ck->off) - nbytes = ck->len - ck->off; + if (nbytes > ck->size - ck->off) + nbytes = ck->size - ck->off; if (nbytes == 0) return (0); @@ -123,7 +191,16 @@ fmemopen_write (void *cookie, const char ck->off += nbytes; - if (ck->off < ck->len && ck->buf[ck->off - 1] != '\0') + if (ck->off > ck->len) + ck->len = ck->off; + + /* + * We append a NULL byte if all these conditions are met: + * - the buffer is not binary + * - the buffer is not full + * - the data just written doesn't already end with a NULL byte + */ + if (!ck->bin && ck->off < ck->size && ck->buf[ck->off - 1] != '\0') ck->buf[ck->off] = '\0'; return (nbytes); @@ -132,12 +209,12 @@ fmemopen_write (void *cookie, const char static fpos_t fmemopen_seek (void *cookie, fpos_t offset, int whence) { - struct __fmemopen_cookie *ck = cookie; + struct fmemopen_cookie *ck = cookie; switch (whence) { case SEEK_SET: - if (offset > ck->len) { + if (offset > ck->size) { errno = EINVAL; return (-1); } @@ -145,7 +222,7 @@ fmemopen_seek (void *cookie, fpos_t offs break; case SEEK_CUR: - if (ck->off + offset > ck->len) { + if (ck->off + offset > ck->size) { errno = EINVAL; return (-1); } @@ -171,7 +248,7 @@ fmemopen_seek (void *cookie, fpos_t offs static int fmemopen_close (void *cookie) { - struct __fmemopen_cookie *ck = cookie; + struct fmemopen_cookie *ck = cookie; if (ck->own) free (ck->buf); Modified: head/lib/libc/stdio/fopen.3 ============================================================================== --- head/lib/libc/stdio/fopen.3 Thu Jan 31 16:04:40 2013 (r246147) +++ head/lib/libc/stdio/fopen.3 Thu Jan 31 16:39:50 2013 (r246148) @@ -118,7 +118,9 @@ after either the or the first letter. This is strictly for compatibility with .St -isoC -and has no effect; the ``b'' is ignored. +and has effect only for +.Fn fmemopen +; otherwise the ``b'' is ignored. .Pp Any created files will have mode .Do Dv S_IRUSR @@ -216,7 +218,7 @@ and arguments with a stream. The .Fa buf -argument shall be either a null pointer or point to a buffer that +argument is either a null pointer or point to a buffer that is at least .Fa size bytes long. @@ -224,10 +226,15 @@ If a null pointer is specified as the .Fa buf argument, .Fn fmemopen -shall allocate +allocates .Fa size -bytes of memory. This buffer shall be automatically freed when the -stream is closed. +bytes of memory. This buffer is automatically freed when the +stream is closed. Buffers can be opened in text-mode (default) or binary-mode +(if ``b'' is present in the second or third position of the +.Fa mode +argument). Buffers opened in text-mode make sure that writes are terminated with +a NULL byte, if the last write hasn't filled up the whole buffer. Buffers +opened in binary-mode never append a NULL byte. .Sh RETURN VALUES Upon successful completion .Fn fopen , @@ -327,3 +334,5 @@ The function conforms to .St -p1003.1-2008 . +The ``b'' mode does not conform to any standard +but is also supported by glibc. Modified: head/tools/regression/lib/libc/stdio/test-fmemopen.c ============================================================================== --- head/tools/regression/lib/libc/stdio/test-fmemopen.c Thu Jan 31 16:04:40 2013 (r246147) +++ head/tools/regression/lib/libc/stdio/test-fmemopen.c Thu Jan 31 16:39:50 2013 (r246148) @@ -41,7 +41,7 @@ void test_preexisting () { /* - * use a pre-existing buffer + * Use a pre-existing buffer. */ char buf[512]; @@ -53,48 +53,52 @@ test_preexisting () size_t nofw, nofr; int rc; - /* open a FILE * using fmemopen */ - fp = fmemopen (buf, sizeof buf, "w"); + /* Open a FILE * using fmemopen. */ + fp = fmemopen (buf, sizeof(buf), "w"); assert (fp != NULL); - /* write to the buffer */ - nofw = fwrite (str, 1, sizeof str, fp); - assert (nofw == sizeof str); + /* Write to the buffer. */ + nofw = fwrite (str, 1, sizeof(str), fp); + assert (nofw == sizeof(str)); - /* close the FILE * */ + /* Close the FILE *. */ rc = fclose (fp); assert (rc == 0); - /* re-open the FILE * to read back the data */ - fp = fmemopen (buf, sizeof buf, "r"); + /* Re-open the FILE * to read back the data. */ + fp = fmemopen (buf, sizeof(buf), "r"); assert (fp != NULL); - /* read from the buffer */ - bzero (buf2, sizeof buf2); - nofr = fread (buf2, 1, sizeof buf2, fp); - assert (nofr == sizeof buf2); + /* Read from the buffer. */ + bzero (buf2, sizeof(buf2)); + nofr = fread (buf2, 1, sizeof(buf2), fp); + assert (nofr == sizeof(buf2)); - /* since a write on a FILE * retrieved by fmemopen + /* + * Since a write on a FILE * retrieved by fmemopen * will add a '\0' (if there's space), we can check - * the strings for equality */ + * the strings for equality. + */ assert (strcmp(str, buf2) == 0); - /* close the FILE * */ + /* Close the FILE *. */ rc = fclose (fp); assert (rc == 0); - /* now open a FILE * on the first 4 bytes of the string */ + /* Now open a FILE * on the first 4 bytes of the string. */ fp = fmemopen (str, 4, "w"); assert (fp != NULL); - /* try to write more bytes than we shoud, we'll get a short count (4) */ - nofw = fwrite (str2, 1, sizeof str2, fp); + /* + * Try to write more bytes than we shoud, we'll get a short count (4). + */ + nofw = fwrite (str2, 1, sizeof(str2), fp); assert (nofw == 4); - /* close the FILE * */ + /* Close the FILE *. */ rc = fclose (fp); - /* check that the string was not modified after the first 4 bytes */ + /* Check that the string was not modified after the first 4 bytes. */ assert (strcmp (str, str3) == 0); } @@ -102,7 +106,7 @@ void test_autoalloc () { /* - * let fmemopen allocate the buffer + * Let fmemopen allocate the buffer. */ char str[] = "A quick test"; @@ -111,8 +115,8 @@ test_autoalloc () size_t nofw, nofr, i; int rc; - /* open a FILE * using fmemopen */ - fp = fmemopen (NULL, 512, "w"); + /* Open a FILE * using fmemopen. */ + fp = fmemopen (NULL, 512, "w+"); assert (fp != NULL); /* fill the buffer */ @@ -121,15 +125,118 @@ test_autoalloc () assert (nofw == 1); } - /* get the current position into the stream */ + /* Get the current position into the stream. */ pos = ftell (fp); assert (pos == 512); - /* try to write past the end, we should get a short object count (0) */ + /* + * Try to write past the end, we should get a short object count (0) + */ nofw = fwrite ("a", 1, 1, fp); assert (nofw == 0); - /* close the FILE * */ + /* Close the FILE *. */ + rc = fclose (fp); + assert (rc == 0); +} + +void +test_data_length () +{ + /* + * Here we test that a read operation doesn't go past the end of the + * data actually written, and that a SEEK_END seeks from the end of the + * data, not of the whole buffer. + */ + FILE *fp; + char buf[512] = {'\0'}; + char str[] = "Test data length. "; + char str2[] = "Do we have two sentences?"; + char str3[sizeof(str) + sizeof(str2) -1]; + long pos; + size_t nofw, nofr; + int rc; + + /* Open a FILE * for updating our buffer. */ + fp = fmemopen (buf, sizeof(buf), "w+"); + assert (fp != NULL); + + /* Write our string into the buffer. */ + nofw = fwrite (str, 1, sizeof(str), fp); + assert (nofw == sizeof(str)); + + /* + * Now seek to the end and check that ftell + * gives us sizeof(str). + */ + rc = fseek (fp, 0, SEEK_END); + assert (rc == 0); + pos = ftell (fp); + assert (pos == sizeof(str)); + + /* Close the FILE *. */ + rc = fclose (fp); + assert (rc == 0); + + /* Reopen the buffer for appending. */ + fp = fmemopen (buf, sizeof(buf), "a+"); + assert (fp != NULL); + + /* We should now be writing after the first string. */ + nofw = fwrite (str2, 1, sizeof(str2), fp); + assert (nofw == sizeof(str2)); + + /* Rewind the FILE *. */ + rc = fseek (fp, 0, SEEK_SET); + assert (rc == 0); + + /* Make sure we're at the beginning. */ + pos = ftell (fp); + assert (pos == 0); + + /* Read the whole buffer. */ + nofr = fread (str3, 1, sizeof(buf), fp); + assert (nofr == sizeof(str3)); + + /* Make sure the two strings are there. */ + assert (strncmp (str3, str, sizeof(str) - 1) == 0); + assert (strncmp (str3 + sizeof(str) - 1, str2, sizeof(str2)) == 0); + + /* Close the FILE *. */ + rc = fclose (fp); + assert (rc == 0); +} + +void +test_binary () +{ + /* + * Make sure that NULL bytes are never appended when opening a buffer + * in binary mode. + */ + + FILE *fp; + char buf[20]; + char str[] = "Test"; + size_t nofw; + int rc, i; + + /* Pre-fill the buffer. */ + memset (buf, 'A', sizeof(buf)); + + /* Open a FILE * in binary mode. */ + fp = fmemopen (buf, sizeof(buf), "w+b"); + assert (fp != NULL); + + /* Write some data into it. */ + nofw = fwrite (str, 1, strlen(str), fp); + assert (nofw == strlen(str)); + + /* Make sure that the buffer doesn't contain any NULL bytes. */ + for (i = 0; i < sizeof(buf); i++) + assert (buf[i] != '\0'); + + /* Close the FILE *. */ rc = fclose (fp); assert (rc == 0); } @@ -139,5 +246,7 @@ main (void) { test_autoalloc (); test_preexisting (); + test_data_length (); + test_binary (); return (0); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201301311639.r0VGdo0Q044244>