mailing list of musl libc
 help / color / mirror / code / Atom feed
* [PATCH] stdio: implement fopencookie(3)
@ 2017-10-10 18:03 William Pitcock
  2017-10-10 18:51 ` Jens Gustedt
  0 siblings, 1 reply; 11+ messages in thread
From: William Pitcock @ 2017-10-10 18:03 UTC (permalink / raw)
  To: musl; +Cc: William Pitcock

The fopencookie(3) function allows the programmer to create a custom
stdio implementation, using four hook functions which operate on a
"cookie" data type.

Changelog:

v3:
- remove spurious `struct winsize`
- make f->lock unconditionally 0

v2:
- properly implement stdio buffering

v1:
- initial proof of concept
---
 include/stdio.h         |  9 +++++
 src/stdio/fopencookie.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 105 insertions(+)
 create mode 100644 src/stdio/fopencookie.c

diff --git a/include/stdio.h b/include/stdio.h
index 884d2e6a..998883e5 100644
--- a/include/stdio.h
+++ b/include/stdio.h
@@ -182,6 +182,15 @@ int vasprintf(char **, const char *, __isoc_va_list);
 #ifdef _GNU_SOURCE
 char *fgets_unlocked(char *, int, FILE *);
 int fputs_unlocked(const char *, FILE *);
+
+typedef struct {
+	ssize_t (*read)(void *cookie, char *buf, size_t size);
+	ssize_t (*write)(void *cookie, const char *buf, size_t size);
+	int (*seek)(void *cookie, off_t *offset, int whence);
+	int (*close)(void *cookie);
+} cookie_io_functions_t;
+
+FILE *fopencookie(void *cookie, const char *mode, cookie_io_functions_t io_funcs);
 #endif
 
 #if defined(_LARGEFILE64_SOURCE) || defined(_GNU_SOURCE)
diff --git a/src/stdio/fopencookie.c b/src/stdio/fopencookie.c
new file mode 100644
index 00000000..baad2585
--- /dev/null
+++ b/src/stdio/fopencookie.c
@@ -0,0 +1,96 @@
+#define _GNU_SOURCE
+#include "stdio_impl.h"
+#include <stdlib.h>
+#include <sys/ioctl.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <string.h>
+
+struct fcookie {
+	void *cookie;
+	cookie_io_functions_t iofuncs;
+};
+
+static size_t cookieread(FILE *f, unsigned char *buf, size_t len)
+{
+	struct fcookie *fc = f->cookie;
+	size_t ret;
+	if (fc->iofuncs.read == NULL) return -1;
+	ret = fc->iofuncs.read(fc->cookie, (char *) buf, len);
+	if (ret == 0) f->flags |= F_EOF;
+	f->rpos = f->buf;
+	f->rend = f->buf + ret;
+	return ret;
+}
+
+static size_t cookiewrite(FILE *f, const unsigned char *buf, size_t len)
+{
+	struct fcookie *fc = f->cookie;
+	size_t ret;
+	size_t len2 = f->wpos - f->wbase;
+	if (fc->iofuncs.write == NULL) return -1;
+	if (len2) {
+		f->wpos = f->wbase;
+		if (cookiewrite(f, f->wpos, len2) < len2) return 0;
+	}
+	return fc->iofuncs.write(fc->cookie, (const char *) buf, len);
+}
+
+static off_t cookieseek(FILE *f, off_t off, int whence)
+{
+	struct fcookie *fc = f->cookie;
+	if (fc->iofuncs.seek) return fc->iofuncs.seek(fc->cookie, &off, whence);
+	return -1;
+}
+
+static int cookieclose(FILE *f)
+{
+	struct fcookie *fc = f->cookie;
+	if (fc->iofuncs.close) return fc->iofuncs.close(fc->cookie);
+	return 0;
+}
+
+FILE *fopencookie(void *cookie, const char *mode, cookie_io_functions_t iofuncs)
+{
+	FILE *f;
+	struct fcookie *fc;
+
+	/* Check for valid initial mode character */
+	if (!strchr("rwa", *mode)) {
+		errno = EINVAL;
+		return 0;
+	}
+
+	/* Allocate FILE+fcookie+buffer or fail */
+	if (!(f=malloc(sizeof *f + sizeof *fc + UNGET + BUFSIZ))) return 0;
+
+	/* Zero-fill only the struct, not the buffer */
+	memset(f, 0, sizeof *f);
+
+	/* Impose mode restrictions */
+	if (!strchr(mode, '+')) f->flags = (*mode == 'r') ? F_NOWR : F_NORD;
+
+	/* Set up our fcookie */
+	fc = (void *)(f + 1);
+	fc->cookie = cookie;
+	fc->iofuncs.read = iofuncs.read;
+	fc->iofuncs.write = iofuncs.write;
+	fc->iofuncs.seek = iofuncs.seek;
+	fc->iofuncs.close = iofuncs.close;
+
+	f->fd = -1;
+	f->cookie = fc;
+	f->buf = (unsigned char *)f + sizeof *f + sizeof *fc + UNGET;
+	f->buf_size = BUFSIZ;
+	f->lbf = EOF;
+	f->lock = 0;
+
+	/* Initialize op ptrs. No problem if some are unneeded. */
+	f->read = cookieread;
+	f->write = cookiewrite;
+	f->seek = cookieseek;
+	f->close = cookieclose;
+
+	/* Add new FILE to open file list */
+	return __ofl_add(f);
+}
-- 
2.13.3



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] stdio: implement fopencookie(3)
  2017-10-10 18:03 [PATCH] stdio: implement fopencookie(3) William Pitcock
@ 2017-10-10 18:51 ` Jens Gustedt
  2017-10-10 20:56   ` Rich Felker
  0 siblings, 1 reply; 11+ messages in thread
From: Jens Gustedt @ 2017-10-10 18:51 UTC (permalink / raw)
  To: William Pitcock; +Cc: musl

[-- Attachment #1: Type: text/plain, Size: 1771 bytes --]

Hello William,

On Tue, 10 Oct 2017 18:03:56 +0000 William Pitcock
<nenolod@dereferenced.org> wrote:

> The fopencookie(3) function allows the programmer to create a custom
> stdio implementation, using four hook functions which operate on a
> "cookie" data type.

I know it is not your fault, but the naming conventions in this new
interface are realy bad design.

> +typedef struct {
> +	ssize_t (*read)(void *cookie, char *buf, size_t size);
> +	ssize_t (*write)(void *cookie, const char *buf, size_t size);
> +	int (*seek)(void *cookie, off_t *offset, int whence);
> +	int (*close)(void *cookie);
> +} cookie_io_functions_t;

> +FILE *fopencookie(void *cookie, const char *mode, cookie_io_functions_t io_funcs);

The members may clash with macro names. E.g an implementation would be
allowed to overload "close" with a macro. This is not possible if the
implementation would want to use this interface here at the same time.

User code could legitimately want to use a macro "seek" for its own
purpose.

Could you at least avoid to use user-space names as function
parameters? Here you should just omit cookie, buf, size, offset,
whence, mode and io_funcs. I think in musl parameters in prototypes
usually don't have names. If you think that we should have them (they
sort of document the interface) you should put them into a reserved
namespace with leading underscore or so, or at least prefix them with
cookie_

Thanks
Jens

-- 
:: INRIA Nancy Grand Est ::: Camus ::::::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536   ::
:: :::::::::::::::::::::: gsm France : +33 651400183   ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] stdio: implement fopencookie(3)
  2017-10-10 18:51 ` Jens Gustedt
@ 2017-10-10 20:56   ` Rich Felker
  2017-10-10 21:40     ` Jens Gustedt
  2017-10-10 22:58     ` Morten Welinder
  0 siblings, 2 replies; 11+ messages in thread
From: Rich Felker @ 2017-10-10 20:56 UTC (permalink / raw)
  To: musl

On Tue, Oct 10, 2017 at 08:51:17PM +0200, Jens Gustedt wrote:
> Hello William,
> 
> On Tue, 10 Oct 2017 18:03:56 +0000 William Pitcock
> <nenolod@dereferenced.org> wrote:
> 
> > The fopencookie(3) function allows the programmer to create a custom
> > stdio implementation, using four hook functions which operate on a
> > "cookie" data type.
> 
> I know it is not your fault, but the naming conventions in this new
> interface are realy bad design.
> 
> > +typedef struct {
> > +	ssize_t (*read)(void *cookie, char *buf, size_t size);
> > +	ssize_t (*write)(void *cookie, const char *buf, size_t size);
> > +	int (*seek)(void *cookie, off_t *offset, int whence);
> > +	int (*close)(void *cookie);
> > +} cookie_io_functions_t;
> 
> > +FILE *fopencookie(void *cookie, const char *mode, cookie_io_functions_t io_funcs);
> 
> The members may clash with macro names. E.g an implementation would be
> allowed to overload "close" with a macro. This is not possible if the
> implementation would want to use this interface here at the same time.
> 
> User code could legitimately want to use a macro "seek" for its own
> purpose.
> 
> Could you at least avoid to use user-space names as function
> parameters? Here you should just omit cookie, buf, size, offset,
> whence, mode and io_funcs. I think in musl parameters in prototypes
> usually don't have names. If you think that we should have them (they
> sort of document the interface) you should put them into a reserved
> namespace with leading underscore or so, or at least prefix them with
> cookie_

I agree with most of the principles here (esp. how bad the public
interface of this function is), but there's not a whole lot that can
be done. Your one request is reasonable and in fact mandatory for musl
header policy: we do not use parameter named at all in prototypes. So
it should read just:

FILE *fopencookie(void *, const char *, cookie_io_functions_t);

Also note that while standard functions in POSIX can additionally be
defined as function-like macros, they can't be object-like macros, so
(*read), etc. are safe due to the parentheses.

Rich


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] stdio: implement fopencookie(3)
  2017-10-10 20:56   ` Rich Felker
@ 2017-10-10 21:40     ` Jens Gustedt
  2017-10-11  2:08       ` Rich Felker
  2017-10-10 22:58     ` Morten Welinder
  1 sibling, 1 reply; 11+ messages in thread
From: Jens Gustedt @ 2017-10-10 21:40 UTC (permalink / raw)
  Cc: musl

[-- Attachment #1: Type: text/plain, Size: 840 bytes --]

Hello Rich,

On Tue, 10 Oct 2017 16:56:54 -0400 Rich Felker <dalias@libc.org> wrote:

> Also note that while standard functions in POSIX can additionally be
> defined as function-like macros, they can't be object-like macros, so
> (*read), etc. are safe due to the parentheses.

They would only be safe in the header. They are not safe on the using
side, I think. Something like

    toto->read = whatever;

or

    *toto = (cookie_io_functions_t){ .read = another, }

can't be protected by parenthesis.

Thanks
Jens

-- 
:: INRIA Nancy Grand Est ::: Camus ::::::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536   ::
:: :::::::::::::::::::::: gsm France : +33 651400183   ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] stdio: implement fopencookie(3)
  2017-10-10 20:56   ` Rich Felker
  2017-10-10 21:40     ` Jens Gustedt
@ 2017-10-10 22:58     ` Morten Welinder
  2017-10-11  2:09       ` Rich Felker
  1 sibling, 1 reply; 11+ messages in thread
From: Morten Welinder @ 2017-10-10 22:58 UTC (permalink / raw)
  To: musl

> Also note that while standard functions in POSIX can additionally be
> defined as function-like macros, they can't be object-like macros, so
> (*read), etc. are safe due to the parentheses.

I'm not sure that's true.  Solaris defines (or used to, at least),
say, "open" to "open64" under the right conditions.  With that, actual
field names in the structure ends up being dependent on #include
order.  Quite a mess and worse for "stat" since "struct stat" gets
renamed in addition to the function.

"#define open my_table.my_open" would be fun too.  I haven't seen that
in the wild, though.

M.






On Tue, Oct 10, 2017 at 4:56 PM, Rich Felker <dalias@libc.org> wrote:
> On Tue, Oct 10, 2017 at 08:51:17PM +0200, Jens Gustedt wrote:
>> Hello William,
>>
>> On Tue, 10 Oct 2017 18:03:56 +0000 William Pitcock
>> <nenolod@dereferenced.org> wrote:
>>
>> > The fopencookie(3) function allows the programmer to create a custom
>> > stdio implementation, using four hook functions which operate on a
>> > "cookie" data type.
>>
>> I know it is not your fault, but the naming conventions in this new
>> interface are realy bad design.
>>
>> > +typedef struct {
>> > +   ssize_t (*read)(void *cookie, char *buf, size_t size);
>> > +   ssize_t (*write)(void *cookie, const char *buf, size_t size);
>> > +   int (*seek)(void *cookie, off_t *offset, int whence);
>> > +   int (*close)(void *cookie);
>> > +} cookie_io_functions_t;
>>
>> > +FILE *fopencookie(void *cookie, const char *mode, cookie_io_functions_t io_funcs);
>>
>> The members may clash with macro names. E.g an implementation would be
>> allowed to overload "close" with a macro. This is not possible if the
>> implementation would want to use this interface here at the same time.
>>
>> User code could legitimately want to use a macro "seek" for its own
>> purpose.
>>
>> Could you at least avoid to use user-space names as function
>> parameters? Here you should just omit cookie, buf, size, offset,
>> whence, mode and io_funcs. I think in musl parameters in prototypes
>> usually don't have names. If you think that we should have them (they
>> sort of document the interface) you should put them into a reserved
>> namespace with leading underscore or so, or at least prefix them with
>> cookie_
>
> I agree with most of the principles here (esp. how bad the public
> interface of this function is), but there's not a whole lot that can
> be done. Your one request is reasonable and in fact mandatory for musl
> header policy: we do not use parameter named at all in prototypes. So
> it should read just:
>
> FILE *fopencookie(void *, const char *, cookie_io_functions_t);
>
> Also note that while standard functions in POSIX can additionally be
> defined as function-like macros, they can't be object-like macros, so
> (*read), etc. are safe due to the parentheses.
>
> Rich


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] stdio: implement fopencookie(3)
  2017-10-10 21:40     ` Jens Gustedt
@ 2017-10-11  2:08       ` Rich Felker
  2017-10-11  5:51         ` Jens Gustedt
  0 siblings, 1 reply; 11+ messages in thread
From: Rich Felker @ 2017-10-11  2:08 UTC (permalink / raw)
  To: musl

On Tue, Oct 10, 2017 at 11:40:15PM +0200, Jens Gustedt wrote:
> Hello Rich,
> 
> On Tue, 10 Oct 2017 16:56:54 -0400 Rich Felker <dalias@libc.org> wrote:
> 
> > Also note that while standard functions in POSIX can additionally be
> > defined as function-like macros, they can't be object-like macros, so
> > (*read), etc. are safe due to the parentheses.
> 
> They would only be safe in the header. They are not safe on the using
> side, I think. Something like
> 
>     toto->read = whatever;
> 
> or
> 
>     *toto = (cookie_io_functions_t){ .read = another, }
> 
> can't be protected by parenthesis.

It doesn't have to be, because it doesn't have the token ( immediately
following it.

Rich


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] stdio: implement fopencookie(3)
  2017-10-10 22:58     ` Morten Welinder
@ 2017-10-11  2:09       ` Rich Felker
  0 siblings, 0 replies; 11+ messages in thread
From: Rich Felker @ 2017-10-11  2:09 UTC (permalink / raw)
  To: musl

On Tue, Oct 10, 2017 at 06:58:30PM -0400, Morten Welinder wrote:
> > Also note that while standard functions in POSIX can additionally be
> > defined as function-like macros, they can't be object-like macros, so
> > (*read), etc. are safe due to the parentheses.
> 
> I'm not sure that's true.  Solaris defines (or used to, at least),
> say, "open" to "open64" under the right conditions.  With that, actual
> field names in the structure ends up being dependent on #include
> order.  Quite a mess and worse for "stat" since "struct stat" gets
> renamed in addition to the function.
> 
> "#define open my_table.my_open" would be fun too.  I haven't seen that
> in the wild, though.

If so that's just a bug in Solaris's headers, for two reasons:

(1) It's an object-like macro not a function-like macro.

(2) It doesn't work if the application does #undef open, which is
explicitly permitted.

Rich


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] stdio: implement fopencookie(3)
  2017-10-11  2:08       ` Rich Felker
@ 2017-10-11  5:51         ` Jens Gustedt
  0 siblings, 0 replies; 11+ messages in thread
From: Jens Gustedt @ 2017-10-11  5:51 UTC (permalink / raw)
  To: musl, Rich Felker

Am 11. Oktober 2017 04:08:08 MESZ schrieb Rich Felker <dalias@libc.org>:
>On Tue, Oct 10, 2017 at 11:40:15PM +0200, Jens Gustedt wrote:
>> Hello Rich,
>> 
>> On Tue, 10 Oct 2017 16:56:54 -0400 Rich Felker <dalias@libc.org>
>wrote:
>> 
>> > Also note that while standard functions in POSIX can additionally
>be
>> > defined as function-like macros, they can't be object-like macros,
>so
>> > (*read), etc. are safe due to the parentheses.
>> 
>> They would only be safe in the header. They are not safe on the using
>> side, I think. Something like
>> 
>>     toto->read = whatever;
>> 
>> or
>> 
>>     *toto = (cookie_io_functions_t){ .read = another, }
>> 
>> can't be protected by parenthesis.
>
>It doesn't have to be, because it doesn't have the token ( immediately
>following it.

I meant this as an example where an object-like macro would hurt. "read" is a bad example, because it is reserved. But if an application had an object-like  macro "seek" it would be screwed.

Jens 

-- 
Jens Gustedt - INRIA & ICube, Strasbourg, France


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] stdio: implement fopencookie(3)
@ 2017-10-10 23:27 William Pitcock
  0 siblings, 0 replies; 11+ messages in thread
From: William Pitcock @ 2017-10-10 23:27 UTC (permalink / raw)
  To: musl; +Cc: William Pitcock

The fopencookie(3) function allows the programmer to create a custom
stdio implementation, using four hook functions which operate on a
"cookie" data type.

Changelog:

v4:
- remove parameter names from header function declarations

v3:
- remove spurious `struct winsize`
- make f->lock unconditionally 0

v2:
- properly implement stdio buffering

v1:
- initial proof of concept
---
 include/stdio.h         |  9 +++++
 src/stdio/fopencookie.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 105 insertions(+)
 create mode 100644 src/stdio/fopencookie.c

diff --git a/include/stdio.h b/include/stdio.h
index 884d2e6a..da0563f6 100644
--- a/include/stdio.h
+++ b/include/stdio.h
@@ -182,6 +182,15 @@ int vasprintf(char **, const char *, __isoc_va_list);
 #ifdef _GNU_SOURCE
 char *fgets_unlocked(char *, int, FILE *);
 int fputs_unlocked(const char *, FILE *);
+
+typedef struct {
+	ssize_t (*read)(void *, char *, size_t);
+	ssize_t (*write)(void *, const char *, size_t);
+	int (*seek)(void *, off_t *, int);
+	int (*close)(void *);
+} cookie_io_functions_t;
+
+FILE *fopencookie(void *, const char *, cookie_io_functions_t);
 #endif
 
 #if defined(_LARGEFILE64_SOURCE) || defined(_GNU_SOURCE)
diff --git a/src/stdio/fopencookie.c b/src/stdio/fopencookie.c
new file mode 100644
index 00000000..baad2585
--- /dev/null
+++ b/src/stdio/fopencookie.c
@@ -0,0 +1,96 @@
+#define _GNU_SOURCE
+#include "stdio_impl.h"
+#include <stdlib.h>
+#include <sys/ioctl.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <string.h>
+
+struct fcookie {
+	void *cookie;
+	cookie_io_functions_t iofuncs;
+};
+
+static size_t cookieread(FILE *f, unsigned char *buf, size_t len)
+{
+	struct fcookie *fc = f->cookie;
+	size_t ret;
+	if (fc->iofuncs.read == NULL) return -1;
+	ret = fc->iofuncs.read(fc->cookie, (char *) buf, len);
+	if (ret == 0) f->flags |= F_EOF;
+	f->rpos = f->buf;
+	f->rend = f->buf + ret;
+	return ret;
+}
+
+static size_t cookiewrite(FILE *f, const unsigned char *buf, size_t len)
+{
+	struct fcookie *fc = f->cookie;
+	size_t ret;
+	size_t len2 = f->wpos - f->wbase;
+	if (fc->iofuncs.write == NULL) return -1;
+	if (len2) {
+		f->wpos = f->wbase;
+		if (cookiewrite(f, f->wpos, len2) < len2) return 0;
+	}
+	return fc->iofuncs.write(fc->cookie, (const char *) buf, len);
+}
+
+static off_t cookieseek(FILE *f, off_t off, int whence)
+{
+	struct fcookie *fc = f->cookie;
+	if (fc->iofuncs.seek) return fc->iofuncs.seek(fc->cookie, &off, whence);
+	return -1;
+}
+
+static int cookieclose(FILE *f)
+{
+	struct fcookie *fc = f->cookie;
+	if (fc->iofuncs.close) return fc->iofuncs.close(fc->cookie);
+	return 0;
+}
+
+FILE *fopencookie(void *cookie, const char *mode, cookie_io_functions_t iofuncs)
+{
+	FILE *f;
+	struct fcookie *fc;
+
+	/* Check for valid initial mode character */
+	if (!strchr("rwa", *mode)) {
+		errno = EINVAL;
+		return 0;
+	}
+
+	/* Allocate FILE+fcookie+buffer or fail */
+	if (!(f=malloc(sizeof *f + sizeof *fc + UNGET + BUFSIZ))) return 0;
+
+	/* Zero-fill only the struct, not the buffer */
+	memset(f, 0, sizeof *f);
+
+	/* Impose mode restrictions */
+	if (!strchr(mode, '+')) f->flags = (*mode == 'r') ? F_NOWR : F_NORD;
+
+	/* Set up our fcookie */
+	fc = (void *)(f + 1);
+	fc->cookie = cookie;
+	fc->iofuncs.read = iofuncs.read;
+	fc->iofuncs.write = iofuncs.write;
+	fc->iofuncs.seek = iofuncs.seek;
+	fc->iofuncs.close = iofuncs.close;
+
+	f->fd = -1;
+	f->cookie = fc;
+	f->buf = (unsigned char *)f + sizeof *f + sizeof *fc + UNGET;
+	f->buf_size = BUFSIZ;
+	f->lbf = EOF;
+	f->lock = 0;
+
+	/* Initialize op ptrs. No problem if some are unneeded. */
+	f->read = cookieread;
+	f->write = cookiewrite;
+	f->seek = cookieseek;
+	f->close = cookieclose;
+
+	/* Add new FILE to open file list */
+	return __ofl_add(f);
+}
-- 
2.13.3



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] stdio: implement fopencookie(3)
  2017-10-05  6:48 William Pitcock
@ 2017-10-05 10:10 ` Szabolcs Nagy
  0 siblings, 0 replies; 11+ messages in thread
From: Szabolcs Nagy @ 2017-10-05 10:10 UTC (permalink / raw)
  To: musl; +Cc: William Pitcock

* William Pitcock <nenolod@dereferenced.org> [2017-10-05 06:48:24 +0000]:
> +FILE *fopencookie(void *cookie, const char *mode, cookie_io_functions_t iofuncs)
> +{
> +	FILE *f;
> +	struct winsize wsz;
> +	struct fcookie *fc;
> +
> +	/* Check for valid initial mode character */
> +	if (!strchr("rwa", *mode)) {
> +		errno = EINVAL;
> +		return 0;
> +	}
> +
> +	/* Allocate FILE+fcookie+buffer or fail */
> +	if (!(f=malloc(sizeof *f + sizeof *fc + UNGET + BUFSIZ))) return 0;
> +
> +	/* Zero-fill only the struct, not the buffer */
> +	memset(f, 0, sizeof *f);
> +
> +	/* Impose mode restrictions */
> +	if (!strchr(mode, '+')) f->flags = (*mode == 'r') ? F_NOWR : F_NORD;
> +
> +	/* Set up our fcookie */
> +	fc = (void *)(f + 1);

note that such malloc and pointer computation
can be problematic if *fc has larger alignment
requirement than *f

> +	fc->cookie = cookie;
> +	fc->iofuncs.read = iofuncs.read;
> +	fc->iofuncs.write = iofuncs.write;
> +	fc->iofuncs.seek = iofuncs.seek;
> +	fc->iofuncs.close = iofuncs.close;
> +
> +	f->fd = -1;
> +	f->cookie = fc;
> +	f->buf = (unsigned char *)f + sizeof *f + sizeof *fc + UNGET;
> +	f->buf_size = BUFSIZ;
> +	f->lbf = EOF;
> +
> +	/* Initialize op ptrs. No problem if some are unneeded. */
> +	f->read = cookieread;
> +	f->write = cookiewrite;
> +	f->seek = cookieseek;
> +	f->close = cookieclose;
> +
> +	if (!libc.threaded) f->lock = -1;
> +

i think this should be unconditional f->lock=0;

if a function uses user callbacks, then it cannot
be assumed that the process remains single-threaded
during the lifetime of that function call.

e.g. with your patch putc can be entered without
locking, then the user write callback may create
a thread that eventually also accesses f before
putc finishes, introducing a data race.

this shows why fopencookie hasnt been implemented
yet: there is no specification what the callbacks
may do, creating a thread is just one example
that messes up the current stdio assumptions,
recursively calling an stdio function on the same
file from a callback may cause deadlock and it's
not clear what should happen.

we can leave the corner-cases unspecified, but
then user code may start to depend on implementation
internals that we cannot change later.

(e.g. recent case in glibc: gnu make used a callback
api for glob, but assumed one of the callbacks are
not used by the implementation so it was not
initialized, now glibc started using that callback
and it took an elaborate api versioning scheme to
avoid breaking old binaries and old make code
compiled against new glibc. similar situation can
arise with fopencookie if one assumes seek is not
called in certain situations in glibc, but then
it turns out musl does call it.)

> +	/* Add new FILE to open file list */
> +	return __ofl_add(f);
> +}
> -- 
> 2.13.3


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] stdio: implement fopencookie(3)
@ 2017-10-05  6:48 William Pitcock
  2017-10-05 10:10 ` Szabolcs Nagy
  0 siblings, 1 reply; 11+ messages in thread
From: William Pitcock @ 2017-10-05  6:48 UTC (permalink / raw)
  To: musl; +Cc: William Pitcock

---
 include/stdio.h         |  9 +++++
 src/stdio/fopencookie.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 107 insertions(+)
 create mode 100644 src/stdio/fopencookie.c

diff --git a/include/stdio.h b/include/stdio.h
index 884d2e6a..998883e5 100644
--- a/include/stdio.h
+++ b/include/stdio.h
@@ -182,6 +182,15 @@ int vasprintf(char **, const char *, __isoc_va_list);
 #ifdef _GNU_SOURCE
 char *fgets_unlocked(char *, int, FILE *);
 int fputs_unlocked(const char *, FILE *);
+
+typedef struct {
+	ssize_t (*read)(void *cookie, char *buf, size_t size);
+	ssize_t (*write)(void *cookie, const char *buf, size_t size);
+	int (*seek)(void *cookie, off_t *offset, int whence);
+	int (*close)(void *cookie);
+} cookie_io_functions_t;
+
+FILE *fopencookie(void *cookie, const char *mode, cookie_io_functions_t io_funcs);
 #endif
 
 #if defined(_LARGEFILE64_SOURCE) || defined(_GNU_SOURCE)
diff --git a/src/stdio/fopencookie.c b/src/stdio/fopencookie.c
new file mode 100644
index 00000000..e0b5a119
--- /dev/null
+++ b/src/stdio/fopencookie.c
@@ -0,0 +1,98 @@
+#define _GNU_SOURCE
+#include "stdio_impl.h"
+#include <stdlib.h>
+#include <sys/ioctl.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <string.h>
+
+struct fcookie {
+	void *cookie;
+	cookie_io_functions_t iofuncs;
+};
+
+static size_t cookieread(FILE *f, unsigned char *buf, size_t len)
+{
+	struct fcookie *fc = f->cookie;
+	size_t ret;
+	if (fc->iofuncs.read == NULL) return -1;
+	ret = fc->iofuncs.read(fc->cookie, (char *) buf, len);
+	if (ret == 0) f->flags |= F_EOF;
+	f->rpos = f->buf;
+	f->rend = f->buf + ret;
+	return ret;
+}
+
+static size_t cookiewrite(FILE *f, const unsigned char *buf, size_t len)
+{
+	struct fcookie *fc = f->cookie;
+	size_t ret;
+	size_t len2 = f->wpos - f->wbase;
+	if (fc->iofuncs.write == NULL) return -1;
+	if (len2) {
+		f->wpos = f->wbase;
+		if (cookiewrite(f, f->wpos, len2) < len2) return 0;
+	}
+	return fc->iofuncs.write(fc->cookie, (const char *) buf, len);
+}
+
+static off_t cookieseek(FILE *f, off_t off, int whence)
+{
+	struct fcookie *fc = f->cookie;
+	if (fc->iofuncs.seek) return fc->iofuncs.seek(fc->cookie, &off, whence);
+	return -1;
+}
+
+static int cookieclose(FILE *f)
+{
+	struct fcookie *fc = f->cookie;
+	if (fc->iofuncs.close) return fc->iofuncs.close(fc->cookie);
+	return 0;
+}
+
+FILE *fopencookie(void *cookie, const char *mode, cookie_io_functions_t iofuncs)
+{
+	FILE *f;
+	struct winsize wsz;
+	struct fcookie *fc;
+
+	/* Check for valid initial mode character */
+	if (!strchr("rwa", *mode)) {
+		errno = EINVAL;
+		return 0;
+	}
+
+	/* Allocate FILE+fcookie+buffer or fail */
+	if (!(f=malloc(sizeof *f + sizeof *fc + UNGET + BUFSIZ))) return 0;
+
+	/* Zero-fill only the struct, not the buffer */
+	memset(f, 0, sizeof *f);
+
+	/* Impose mode restrictions */
+	if (!strchr(mode, '+')) f->flags = (*mode == 'r') ? F_NOWR : F_NORD;
+
+	/* Set up our fcookie */
+	fc = (void *)(f + 1);
+	fc->cookie = cookie;
+	fc->iofuncs.read = iofuncs.read;
+	fc->iofuncs.write = iofuncs.write;
+	fc->iofuncs.seek = iofuncs.seek;
+	fc->iofuncs.close = iofuncs.close;
+
+	f->fd = -1;
+	f->cookie = fc;
+	f->buf = (unsigned char *)f + sizeof *f + sizeof *fc + UNGET;
+	f->buf_size = BUFSIZ;
+	f->lbf = EOF;
+
+	/* Initialize op ptrs. No problem if some are unneeded. */
+	f->read = cookieread;
+	f->write = cookiewrite;
+	f->seek = cookieseek;
+	f->close = cookieclose;
+
+	if (!libc.threaded) f->lock = -1;
+
+	/* Add new FILE to open file list */
+	return __ofl_add(f);
+}
-- 
2.13.3



^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-10-11  5:51 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-10 18:03 [PATCH] stdio: implement fopencookie(3) William Pitcock
2017-10-10 18:51 ` Jens Gustedt
2017-10-10 20:56   ` Rich Felker
2017-10-10 21:40     ` Jens Gustedt
2017-10-11  2:08       ` Rich Felker
2017-10-11  5:51         ` Jens Gustedt
2017-10-10 22:58     ` Morten Welinder
2017-10-11  2:09       ` Rich Felker
  -- strict thread matches above, loose matches on Subject: below --
2017-10-10 23:27 William Pitcock
2017-10-05  6:48 William Pitcock
2017-10-05 10:10 ` Szabolcs Nagy

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).