On Wed, Jan 30, 2013 at 02:59:26PM +0000, Pietro Cerutti wrote:
> Author: gahr (ports committer)
> Date: Wed Jan 30 14:59:26 2013
> New Revision: 246120
> URL: http://svnweb.freebsd.org/changeset/base/246120

> Log:
>   Add fmemopen(3), an interface to get a FILE * from a buffer in memory, along
>   with the respective regression test.
>   See http://pubs.opengroup.org/onlinepubs/9699919799/functions/fmemopen.html

Various comments are inline.

> Added:
>   head/lib/libc/stdio/fmemopen.c   (contents, props changed)
>   head/tools/regression/lib/libc/stdio/test-fmemopen.c   (contents, props 
> changed)
>   head/tools/regression/lib/libc/stdio/test-fmemopen.t   (contents, props 
> changed)
> Modified:
>   head/include/stdio.h   (contents, props changed)
>   head/lib/libc/stdio/Makefile.inc   (contents, props changed)
>   head/lib/libc/stdio/Symbol.map   (contents, props changed)
>   head/lib/libc/stdio/fopen.3   (contents, props changed)

> Modified: head/lib/libc/stdio/Makefile.inc
> ==============================================================================
> --- head/lib/libc/stdio/Makefile.inc  Wed Jan 30 13:14:34 2013        
> (r246119)
> +++ head/lib/libc/stdio/Makefile.inc  Wed Jan 30 14:59:26 2013        
> (r246120)
> @@ -8,7 +8,8 @@ SRCS+=        _flock_stub.c asprintf.c clrerr.c
>       fclose.c fcloseall.c fdopen.c \
>       feof.c ferror.c fflush.c fgetc.c fgetln.c fgetpos.c fgets.c fgetwc.c \
>       fgetwln.c fgetws.c \
> -     fileno.c findfp.c flags.c fopen.c fprintf.c fpurge.c fputc.c fputs.c \
> +     fileno.c findfp.c flags.c fmemopen.c fopen.c fprintf.c fpurge.c \
> +     fputc.c fputs.c \
>       fputwc.c fputws.c fread.c freopen.c fscanf.c fseek.c fsetpos.c \
>       ftell.c funopen.c fvwrite.c fwalk.c fwide.c fwprintf.c fwscanf.c \
>       fwrite.c getc.c getchar.c getdelim.c getline.c \
> @@ -48,7 +49,7 @@ MLINKS+=ferror.3 ferror_unlocked.3 \
>  MLINKS+=fflush.3 fpurge.3
>  MLINKS+=fgets.3 gets.3
>  MLINKS+=flockfile.3 ftrylockfile.3 flockfile.3 funlockfile.3
> -MLINKS+=fopen.3 fdopen.3 fopen.3 freopen.3
> +MLINKS+=fopen.3 fdopen.3 fopen.3 freopen.3 fopen.3 fmemopen.3
>  MLINKS+=fputs.3 puts.3
>  MLINKS+=fread.3 fwrite.3
>  MLINKS+=fseek.3 fgetpos.3 fseek.3 fseeko.3 fseek.3 fsetpos.3 fseek.3 ftell.3 
> \

I think the man page could be a separate page without problem. There is
a fair amount of behaviour unique to fmemopen().

> Added: head/lib/libc/stdio/fmemopen.c
> ==============================================================================
> --- /dev/null 00:00:00 1970   (empty, because file is newly added)
> +++ head/lib/libc/stdio/fmemopen.c    Wed Jan 30 14:59:26 2013        
> (r246120)
> @@ -0,0 +1,182 @@
> +/*-
> +Copyright (C) 2013 Pietro Cerutti <g...@freebsd.org>
> +
> +Redistribution and use in source and binary forms, with or without
> +modification, are permitted provided that the following conditions
> +are met:
> +1. Redistributions of source code must retain the above copyright
> +   notice, this list of conditions and the following disclaimer.
> +2. Redistributions in binary form must reproduce the above copyright
> +   notice, this list of conditions and the following disclaimer in the
> +   documentation and/or other materials provided with the distribution.
> +
> +THIS SOFTWARE IS PROVIDED BY AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> +ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> +IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> +ARE DISCLAIMED.  IN NO EVENT SHALL AUTHOR OR CONTRIBUTORS BE LIABLE
> +FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> +DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> +OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> +HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> +LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> +OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> +SUCH DAMAGE.
> +*/
> +
> +#include <sys/cdefs.h>
> +__FBSDID("$FreeBSD$");
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <errno.h>
> +
> +struct __fmemopen_cookie

I think the underscores are not necessary if the name cannot collide
with a user's name.

> +{
> +     char *buf;      /* pointer to the memory region */
> +     char  own;      /* did we allocate the buffer ourselves? */

> +     long  len;      /* buffer length in bytes */

This is in fact a size_t, as passed to fmemopen().

> +     long  off;      /* current offset into the buffer */

This should probably be a size_t as well.

> +};
> +
> +static int   fmemopen_read  (void *cookie, char *buf, int nbytes);
> +static int   fmemopen_write (void *cookie, const char *buf, int nbytes);
> +static fpos_t        fmemopen_seek  (void *cookie, fpos_t offset, int 
> whence);
> +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));
> +     if (ck == NULL) {
> +             errno = ENOMEM;

malloc() already sets errno when it fails; you need not do it yourself.

> +             return (NULL);
> +     }
> +
> +     ck->off = 0;
> +     ck->len = size;
> +
> +     /* do 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);
> +             }
> +             ck->buf[0] = '\0';
> +     }
> +
> +     if (mode[0] == 'a')
> +             ck->off = strnlen(ck->buf, ck->len);

This is the only use of 'mode'. POSIX mentions a difference between 'r'
and 'w' with regard to another piece of internal state: the notion of
the size of the current buffer contents, which seems completely missing
here. There was an interpretation issued about that as well,
http://austingroupbugs.net/view.php?id=587 .

glibc does something different when the mode specifies binary ('b') and
POSIX will likely require this in future versions, see
http://austingroupbugs.net/view.php?id=456 . It is probably best to fail
with [EINVAL] if this special behaviour is not implemented so that
people are not surprised.

> +
> +     /* actuall wrapper */
> +     FILE *f = funopen ((void *)ck, fmemopen_read, fmemopen_write,
> +         fmemopen_seek, fmemopen_close);

The read and write functions should be passed as NULL instead when the
mode does not permit reading or writing respectively.

> +
> +     if (f == NULL) {
> +             if (ck->own)
> +                     free (ck->buf);
> +             free (ck);
> +             return (NULL);
> +     }
> +
> +     /* turn off buffering, so a write past the end of the buffer
> +      * correctly returns a short object count */
> +     setvbuf (f, (char *) NULL, _IONBF, 0);

This is indeed required but I think it makes the stream rather slow
(because it will call fmemopen_read/fmemopen_write for single bytes). If
this is a problem, I think it should be fixed by making unbuffered mode
smarter in the rest of stdio and not here.

> +
> +     return (f);
> +}
> +
> +static int
> +fmemopen_read (void *cookie, char *buf, int nbytes)

stdio is strange, requiring 'int' here :(

> +{
> +     struct __fmemopen_cookie *ck = cookie;
> +
> +     if (nbytes > ck->len - ck->off)
> +             nbytes = ck->len - ck->off;
> +
> +     if (nbytes == 0)
> +             return (0);
> +
> +     memcpy (buf, ck->buf + ck->off, nbytes);
> +
> +     ck->off += nbytes;
> +
> +     return (nbytes);
> +}
> [snip]
> Modified: head/lib/libc/stdio/fopen.3
> ==============================================================================
> --- head/lib/libc/stdio/fopen.3       Wed Jan 30 13:14:34 2013        
> (r246119)
> +++ head/lib/libc/stdio/fopen.3       Wed Jan 30 14:59:26 2013        
> (r246120)
> @@ -32,13 +32,14 @@
> [snip]
> @@ -202,6 +205,29 @@ standard text stream
>  .Dv ( stderr , stdin ,
>  or
>  .Dv stdout ) .
> +.Pp
> +The
> +.Fn fmemopen
> +function
> +associates the buffer given by the
> +.Fa buf
> +and
> +.Fa size
> +arguments with a stream.
> +The
> +.Fa buf
> +argument shall be either a null pointer or point to a buffer that
> +is at least
> +.Fa size
> +bytes long.
> +If a null pointer is specified as the
> +.Fa buf
> +argument,
> +.Fn fmemopen
> +shall allocate

We don't usually use 'shall' for requirements on the implementation in
man pages. We made it such that it is compliant; thus, it is, not shall
be.

> [snip]
> @@ -294,3 +322,8 @@ The
>  .Dq Li e
>  mode option does not conform to any standard
>  but is also supported by glibc.
> +The
> +.Fn fmemopen
> +function
> +conforms to 
> +.St -p1003.1-2008 .

Quite a bit of functionality is missing.

> Added: head/tools/regression/lib/libc/stdio/test-fmemopen.c
> ==============================================================================
> --- /dev/null 00:00:00 1970   (empty, because file is newly added)
> +++ head/tools/regression/lib/libc/stdio/test-fmemopen.c      Wed Jan 30 
> 14:59:26 2013        (r246120)
> @@ -0,0 +1,143 @@
> +/*-
> +Copyright (C) 2013 Pietro Cerutti <g...@freebsd.org>
> +
> +Redistribution and use in source and binary forms, with or without
> +modification, are permitted provided that the following conditions
> +are met:
> +1. Redistributions of source code must retain the above copyright
> +   notice, this list of conditions and the following disclaimer.
> +2. Redistributions in binary form must reproduce the above copyright
> +   notice, this list of conditions and the following disclaimer in the
> +   documentation and/or other materials provided with the distribution.
> +
> +THIS SOFTWARE IS PROVIDED BY AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> +ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> +IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> +ARE DISCLAIMED.  IN NO EVENT SHALL AUTHOR OR CONTRIBUTORS BE LIABLE
> +FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> +DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> +OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> +HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> +LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> +OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> +SUCH DAMAGE.
> +*/
> +
> +/*
> + * Test basic FILE * functions (fread, fwrite, fseek, fclose) against
> + * a FILE * retrieved using fmemopen()
> + */
> +
> +#include <sys/cdefs.h>
> +__FBSDID("$FreeBSD$");
> +
> +#include <assert.h>
> +#include <errno.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <strings.h>
> +
> [snip]
> +void
> +test_autoalloc ()
> +{
> +     /* 
> +      * let fmemopen allocate the buffer
> +      */
> +
> +     char str[] = "A quick test";
> +     FILE *fp;
> +     long pos;
> +     size_t nofw, nofr, i;
> +     int rc;
> +
> +     /* open a FILE * using fmemopen */
> +     fp = fmemopen (NULL, 512, "w");

This does not make sense in a real program: there is no way to read the
data back. POSIX permits fmemopen(NULL, ...) to fail when the mode is
read-only or write-only.

> +     assert (fp != NULL);
> +
> +     /* fill the buffer */
> +     for (i = 0; i < 512; i++) {
> +             nofw = fwrite ("a", 1, 1, fp);
> +             assert (nofw == 1);
> +     }
> +
> +     /* 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) */
> +     nofw = fwrite ("a", 1, 1, fp);
> +     assert (nofw == 0);
> +
> +     /* close the FILE * */
> +     rc = fclose (fp);
> +     assert (rc == 0);
> +}
> [snip]

-- 
Jilles Tjoelker
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to