Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 1 Jun 2011 17:36:52 +0000 (UTC)
From:      Matthew D Fleming <mdf@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org
Subject:   svn commit: r222574 - in stable/8: share/man/man9 sys/kern sys/sys
Message-ID:  <201106011736.p51HaqTu079955@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: mdf
Date: Wed Jun  1 17:36:52 2011
New Revision: 222574
URL: http://svn.freebsd.org/changeset/base/222574

Log:
  MFC r212365, r212367:
  
  r212365: Refactor sbuf code so that most uses of sbuf_extend() are in
  a new sbuf_put_byte().  This makes it easier to add drain
  functionality when a buffer would overflow as there are fewer code
  points.
  
  r212367: Add drain functionality to sbufs.  The drain is a function
  that is called when the sbuf internal buffer is filled.  For kernel
  sbufs with a drain, the internal buffer will never be expanded.  For
  userland sbufs with a drain, the internal buffer may still be expanded
  by sbuf_[v]printf(3).
  
  Sbufs now have three basic uses:
  1) static string manipulation.  Overflow is marked.
  2) dynamic string manipulation.  Overflow triggers string growth.
  3) drained string manipulation.  Overflow triggers draining.
  
  In all cases the manipulation is 'safe' in that overflow is detected and
  managed.
  
  Note that r212367 had to be minorly re-implemented to dynamically
  allocate space for a function pointer, a void * argument, and an int
  error, to not break the ABI/KBI.

Modified:
  stable/8/share/man/man9/Makefile
  stable/8/share/man/man9/sbuf.9
  stable/8/sys/kern/subr_sbuf.c
  stable/8/sys/sys/sbuf.h
Directory Properties:
  stable/8/share/man/man9/   (props changed)
  stable/8/sys/   (props changed)
  stable/8/sys/amd64/include/xen/   (props changed)
  stable/8/sys/cddl/contrib/opensolaris/   (props changed)
  stable/8/sys/contrib/dev/acpica/   (props changed)
  stable/8/sys/contrib/pf/   (props changed)

Modified: stable/8/share/man/man9/Makefile
==============================================================================
--- stable/8/share/man/man9/Makefile	Wed Jun  1 17:17:02 2011	(r222573)
+++ stable/8/share/man/man9/Makefile	Wed Jun  1 17:36:52 2011	(r222574)
@@ -1028,6 +1028,7 @@ MLINKS+=sbuf.9 sbuf_bcat.9 \
 	sbuf.9 sbuf_overflowed.9 \
 	sbuf.9 sbuf_printf.9 \
 	sbuf.9 sbuf_putc.9 \
+	sbuf.9 sbuf_set_drain.9 \
 	sbuf.9 sbuf_setpos.9 \
 	sbuf.9 sbuf_trim.9 \
 	sbuf.9 sbuf_vprintf.9

Modified: stable/8/share/man/man9/sbuf.9
==============================================================================
--- stable/8/share/man/man9/sbuf.9	Wed Jun  1 17:17:02 2011	(r222573)
+++ stable/8/share/man/man9/sbuf.9	Wed Jun  1 17:36:52 2011	(r222574)
@@ -43,6 +43,7 @@
 .Nm sbuf_printf ,
 .Nm sbuf_vprintf ,
 .Nm sbuf_putc ,
+.Nm sbuf_set_drain ,
 .Nm sbuf_trim ,
 .Nm sbuf_overflowed ,
 .Nm sbuf_finish ,
@@ -54,6 +55,8 @@
 .Sh SYNOPSIS
 .In sys/types.h
 .In sys/sbuf.h
+.Ft typedef\ int ( sbuf_drain_func ) ( void\ *arg, const\ char\ *data, int\ len ) ;
+.Pp
 .Ft struct sbuf *
 .Fn sbuf_new "struct sbuf *s" "char *buf" "int length" "int flags"
 .Ft struct sbuf *
@@ -80,11 +83,13 @@
 .Fn sbuf_vprintf "struct sbuf *s" "const char *fmt" "va_list ap"
 .Ft int
 .Fn sbuf_putc "struct sbuf *s" "int c"
+.Ft void
+.Fn sbuf_set_drain "struct sbuf *s" "sbuf_drain_func *func" "void *arg"
 .Ft int
 .Fn sbuf_trim "struct sbuf *s"
 .Ft int
 .Fn sbuf_overflowed "struct sbuf *s"
-.Ft void
+.Ft int
 .Fn sbuf_finish "struct sbuf *s"
 .Ft char *
 .Fn sbuf_data "struct sbuf *s"
@@ -224,6 +229,51 @@ to the
 at the current position.
 .Pp
 The
+.Fn sbuf_set_drain
+function sets a drain function
+.Fa func
+for the
+.Fa sbuf ,
+and records a pointer
+.Fa arg
+to be passed to the drain on callback.
+The drain function cannot be changed while
+.Fa sbuf_len
+is non-zero.
+.Pp
+The registered drain function
+.Vt sbuf_drain_func
+will be called with the argument
+.Fa arg
+provided to
+.Fn sbuf_set_drain ,
+a pointer
+.Fa data
+to a byte string that is the contents of the sbuf, and the length
+.Fa len
+of the data.
+If the drain function exists, it will be called when the sbuf internal
+buffer is full, or on behalf of
+.Fn sbuf_finish .
+The drain function may drain some or all of the data, but must drain
+at least 1 byte.
+The return value from the drain function, if positive, indicates how
+many bytes were drained.
+If negative, the return value indicates the negative error code which
+will be returned from this or a later call to
+.Fn sbuf_finish .
+The returned drained length cannot be zero.
+To do unbuffered draining, initialize the sbuf with a two-byte buffer.
+The drain will be called for every byte added to the sbuf.
+The
+.Fn sbuf_bcopyin ,
+.Fn sbuf_copyin ,
+.Fn sbuf_trim ,
+and
+.Fn sbuf_data
+functions cannot be used on an sbuf with a drain.
+.Pp
+The
 .Fn sbuf_copyin
 function copies a NUL-terminated string from the specified userland
 address into the
@@ -289,10 +339,17 @@ overflowed.
 .Pp
 The
 .Fn sbuf_finish
-function NUL-terminates the
+function will call the attached drain function if one exists until all
+the data in the
 .Fa sbuf
-and marks it as finished, which means that it may no longer be
-modified using
+is flushed.
+If there is no attached drain,
+.Fn sbuf_finish
+NUL-terminates the
+.Fa sbuf .
+In either case it marks the
+.Fa sbuf
+as finished, which means that it may no longer be modified using
 .Fn sbuf_setpos ,
 .Fn sbuf_cat ,
 .Fn sbuf_cpy ,
@@ -305,12 +362,21 @@ is used to reset the sbuf.
 .Pp
 The
 .Fn sbuf_data
-and
-.Fn sbuf_len
-functions return the actual string and its length, respectively;
+function returns the actual string;
 .Fn sbuf_data
 only works on a finished
 .Fa sbuf .
+The
+.Fn sbuf_len function returns the length of the string.
+For an
+.Fa sbuf
+with an attached drain,
+.Fn sbuf_len
+returns the length of the un-drained data.
+.Fn sbuf_done
+returns non-zero if the
+.Fa sbuf
+is finished.
 .Fn sbuf_done
 returns non-zero if the
 .Fa sbuf
@@ -329,6 +395,22 @@ size of its storage buffer using
 .Fn sbuf_setpos ,
 or it is reinitialized to a sufficiently short string using
 .Fn sbuf_cpy .
+.Pp
+Drains in user-space will not always function as indicated.
+While the drain function will be called immediately on overflow from
+the
+.Fa sbuf_putc ,
+.Fa sbuf_bcat ,
+.Fa sbuf_cat
+functions,
+.Fa sbuf_printf
+and
+.Fa sbuf_vprintf
+currently have no way to determine whether there will be an overflow
+until after it occurs, and cannot do a partial expansion of the format
+string.
+Thus when using libsbuf the buffer may be extended to allow completion
+of a single printf call, even though a drain is attached.
 .Sh RETURN VALUES
 The
 .Fn sbuf_new
@@ -372,6 +454,14 @@ The
 function
 returns \-1 if copying string from userland failed, and number of bytes
 copied otherwise.
+The
+.Fn sbuf_finish
+function returns ENOMEM if the sbuf overflowed before being finished,
+or returns the error code from the drain if one is attached.
+When used as
+.Xr sbuf_finish 3 ,
+.Fn sbuf_finish
+will return \-1 and set errno on error instead.
 .Sh SEE ALSO
 .Xr printf 3 ,
 .Xr strcat 3 ,
@@ -396,6 +486,8 @@ Additional improvements were suggested b
 .An Justin T. Gibbs Aq gibbs@FreeBSD.org .
 Auto-extend support added by
 .An Kelly Yancey Aq kbyanc@FreeBSD.org .
+Drain functionality added by
+.An Matthew Fleming Aq mdf@FreeBSD.org .
 .Pp
 This manual page was written by
 .An Dag-Erling Sm\(/orgrav Aq des@FreeBSD.org .

Modified: stable/8/sys/kern/subr_sbuf.c
==============================================================================
--- stable/8/sys/kern/subr_sbuf.c	Wed Jun  1 17:17:02 2011	(r222573)
+++ stable/8/sys/kern/subr_sbuf.c	Wed Jun  1 17:36:52 2011	(r222574)
@@ -33,6 +33,7 @@ __FBSDID("$FreeBSD$");
 
 #ifdef _KERNEL
 #include <sys/ctype.h>
+#include <sys/errno.h>
 #include <sys/kernel.h>
 #include <sys/malloc.h>
 #include <sys/systm.h>
@@ -40,6 +41,7 @@ __FBSDID("$FreeBSD$");
 #include <machine/stdarg.h>
 #else /* _KERNEL */
 #include <ctype.h>
+#include <errno.h>
 #include <stdarg.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -48,6 +50,12 @@ __FBSDID("$FreeBSD$");
 
 #include <sys/sbuf.h>
 
+struct sbuf_drain {
+	sbuf_drain_func	*s_func;	/* drain function */
+	void		*s_arg;		/* user-supplied drain argument */
+	int		 s_error;	/* current error code */
+};
+
 #ifdef _KERNEL
 static MALLOC_DEFINE(M_SBUF, "sbuf", "string buffers");
 #define	SBMALLOC(size)		malloc(size, M_SBUF, M_WAITOK)
@@ -246,6 +254,8 @@ sbuf_clear(struct sbuf *s)
 
 	SBUF_CLEARFLAG(s, SBUF_FINISHED);
 	SBUF_CLEARFLAG(s, SBUF_OVERFLOWED);
+	if (s->s_drain != NULL)
+		s->s_drain->s_error = 0;
 	s->s_len = 0;
 }
 
@@ -272,27 +282,125 @@ sbuf_setpos(struct sbuf *s, int pos)
 }
 
 /*
+ * Set up a drain function and argument on an sbuf to flush data to
+ * when the sbuf buffer overflows.
+ */
+void
+sbuf_set_drain(struct sbuf *s, sbuf_drain_func *func, void *ctx)
+{
+
+	assert_sbuf_state(s, 0);
+	assert_sbuf_integrity(s);
+	KASSERT((s->s_drain != NULL && func == s->s_drain->s_func) ||
+	    s->s_len == 0,
+	    ("Cannot change drain to %p on non-empty sbuf %p", func, s));
+	if (func == NULL) {
+		SBFREE(s->s_drain);
+		s->s_drain = NULL;
+		return;
+	}
+	if (s->s_drain == NULL) {
+		s->s_drain = SBMALLOC(sizeof(*s->s_drain));
+		if (s->s_drain == NULL)
+			return;
+	}
+	s->s_drain->s_func = func;
+	s->s_drain->s_arg = ctx;
+	s->s_drain->s_error = 0;
+}
+
+/*
+ * Call the drain and process the return.
+ */
+static int
+sbuf_drain(struct sbuf *s)
+{
+	int len;
+
+	KASSERT(s->s_len > 0, ("Shouldn't drain empty sbuf %p", s));
+	len = s->s_drain->s_func(s->s_drain->s_arg, s->s_buf, s->s_len);
+	if (len < 0) {
+		s->s_drain->s_error = -len;
+		SBUF_SETFLAG(s, SBUF_OVERFLOWED);
+		return (s->s_drain->s_error);
+	}
+
+	KASSERT(len > 0, ("Drain must either error or work!"));
+	s->s_len -= len;
+	/*
+	 * Fast path for the expected case where all the data was
+	 * drained.
+	 */
+	if (s->s_len == 0)
+		return (0);
+	/*
+	 * Move the remaining characters to the beginning of the
+	 * string.
+	 */
+	memmove(s->s_buf, s->s_buf + len, s->s_len);
+	return (0);
+}
+
+/*
+ * Append a byte to an sbuf.  This is the core function for appending
+ * to an sbuf and is the main place that deals with extending the
+ * buffer and marking overflow.
+ */
+static void
+sbuf_put_byte(int c, struct sbuf *s)
+{
+
+	assert_sbuf_integrity(s);
+	assert_sbuf_state(s, 0);
+
+	if (SBUF_HASOVERFLOWED(s))
+		return;
+	if (SBUF_FREESPACE(s) <= 0) {
+		/* 
+		 * If there is a drain, use it, otherwise extend the
+		 * buffer.
+		 */
+		if (s->s_drain != NULL)
+			(void)sbuf_drain(s);
+		else if (sbuf_extend(s, 1) < 0)
+			SBUF_SETFLAG(s, SBUF_OVERFLOWED);
+		if (SBUF_HASOVERFLOWED(s))
+			return;
+	}
+	s->s_buf[s->s_len++] = c;
+}
+
+/*
+ * Append a non-NUL character to an sbuf.  This prototype signature is
+ * suitable for use with kvprintf(9).
+ */
+static void
+sbuf_putc_func(int c, void *arg)
+{
+
+	if (c != '\0')
+		sbuf_put_byte(c, arg);
+}
+
+/*
  * Append a byte string to an sbuf.
  */
 int
 sbuf_bcat(struct sbuf *s, const void *buf, size_t len)
 {
 	const char *str = buf;
+	const char *end = str + len;
 
 	assert_sbuf_integrity(s);
 	assert_sbuf_state(s, 0);
 
 	if (SBUF_HASOVERFLOWED(s))
 		return (-1);
-	for (; len; len--) {
-		if (!SBUF_HASROOM(s) && sbuf_extend(s, len) < 0)
-			break;
-		s->s_buf[s->s_len++] = *str++;
-	}
-	if (len > 0) {
-		SBUF_SETFLAG(s, SBUF_OVERFLOWED);
-		return (-1);
-	}
+	for (; str < end; str++) {
+		sbuf_put_byte(*str, s);
+		if (SBUF_HASOVERFLOWED(s))
+			return (-1);
+ 	}
 	return (0);
 }
 
@@ -306,6 +414,8 @@ sbuf_bcopyin(struct sbuf *s, const void 
 
 	assert_sbuf_integrity(s);
 	assert_sbuf_state(s, 0);
+	KASSERT(s->s_drain == NULL,
+	    ("Nonsensical copyin to sbuf %p with a drain", s));
 
 	if (SBUF_HASOVERFLOWED(s))
 		return (-1);
@@ -352,13 +462,9 @@ sbuf_cat(struct sbuf *s, const char *str
 		return (-1);
 
 	while (*str != '\0') {
-		if (!SBUF_HASROOM(s) && sbuf_extend(s, strlen(str)) < 0)
-			break;
-		s->s_buf[s->s_len++] = *str++;
-	}
-	if (*str != '\0') {
-		SBUF_SETFLAG(s, SBUF_OVERFLOWED);
-		return (-1);
+		sbuf_put_byte(*str, s);
+		if (SBUF_HASOVERFLOWED(s))
+			return (-1);
 	}
 	return (0);
 }
@@ -374,6 +480,8 @@ sbuf_copyin(struct sbuf *s, const void *
 
 	assert_sbuf_integrity(s);
 	assert_sbuf_state(s, 0);
+	KASSERT(s->s_drain == NULL,
+	    ("Nonsensical copyin to sbuf %p with a drain", s));
 
 	if (SBUF_HASOVERFLOWED(s))
 		return (-1);
@@ -417,11 +525,28 @@ sbuf_cpy(struct sbuf *s, const char *str
 /*
  * Format the given argument list and append the resulting string to an sbuf.
  */
+#ifdef _KERNEL
+int
+sbuf_vprintf(struct sbuf *s, const char *fmt, va_list ap)
+{
+
+	assert_sbuf_integrity(s);
+	assert_sbuf_state(s, 0);
+
+	KASSERT(fmt != NULL,
+	    ("%s called with a NULL format string", __func__));
+
+	(void)kvprintf(fmt, sbuf_putc_func, s, 10, ap);
+	if (SBUF_HASOVERFLOWED(s))
+		return (-1);
+	return (0);
+}
+#else /* !_KERNEL */
 int
 sbuf_vprintf(struct sbuf *s, const char *fmt, va_list ap)
 {
 	va_list ap_copy;
-	int len;
+	int error, len;
 
 	assert_sbuf_integrity(s);
 	assert_sbuf_state(s, 0);
@@ -432,13 +557,32 @@ sbuf_vprintf(struct sbuf *s, const char 
 	if (SBUF_HASOVERFLOWED(s))
 		return (-1);
 
+	/*
+	 * For the moment, there is no way to get vsnprintf(3) to hand
+	 * back a character at a time, to push everything into
+	 * sbuf_putc_func() as was done for the kernel.
+	 *
+	 * In userspace, while drains are useful, there's generally
+	 * not a problem attempting to malloc(3) on out of space.  So
+	 * expand a userland sbuf if there is not enough room for the
+	 * data produced by sbuf_[v]printf(3).
+	 */
+
+	error = 0;
 	do {
 		va_copy(ap_copy, ap);
 		len = vsnprintf(&s->s_buf[s->s_len], SBUF_FREESPACE(s) + 1,
 		    fmt, ap_copy);
 		va_end(ap_copy);
-	} while (len > SBUF_FREESPACE(s) &&
-	    sbuf_extend(s, len - SBUF_FREESPACE(s)) == 0);
+
+		if (SBUF_FREESPACE(s) >= len)
+			break;
+		/* Cannot print with the current available space. */
+		if (s->s_drain != NULL && s->s_len > 0)
+			error = sbuf_drain(s);
+		else
+			error = sbuf_extend(s, len - SBUF_FREESPACE(s));
+	} while (error == 0);
 
 	/*
 	 * s->s_len is the length of the string, without the terminating nul.
@@ -462,6 +606,7 @@ sbuf_vprintf(struct sbuf *s, const char 
 		return (-1);
 	return (0);
 }
+#endif /* _KERNEL */
 
 /*
  * Format the given arguments and append the resulting string to an sbuf.
@@ -485,17 +630,9 @@ int
 sbuf_putc(struct sbuf *s, int c)
 {
 
-	assert_sbuf_integrity(s);
-	assert_sbuf_state(s, 0);
-
+	sbuf_putc_func(c, s);
 	if (SBUF_HASOVERFLOWED(s))
 		return (-1);
-	if (!SBUF_HASROOM(s) && sbuf_extend(s, 1) < 0) {
-		SBUF_SETFLAG(s, SBUF_OVERFLOWED);
-		return (-1);
-	}
-	if (c != '\0')
-		s->s_buf[s->s_len++] = c;
 	return (0);
 }
 
@@ -508,6 +645,8 @@ sbuf_trim(struct sbuf *s)
 
 	assert_sbuf_integrity(s);
 	assert_sbuf_state(s, 0);
+	KASSERT(s->s_drain == NULL,
+	    ("%s makes no sense on sbuf %p with drain", __func__, s));
 
 	if (SBUF_HASOVERFLOWED(s))
 		return (-1);
@@ -531,16 +670,32 @@ sbuf_overflowed(struct sbuf *s)
 /*
  * Finish off an sbuf.
  */
-void
+int
 sbuf_finish(struct sbuf *s)
 {
+	int error = 0;
 
 	assert_sbuf_integrity(s);
 	assert_sbuf_state(s, 0);
 
+	if (s->s_drain != NULL) {
+		error = s->s_drain->s_error;
+		while (s->s_len > 0 && error == 0)
+			error = sbuf_drain(s);
+	} else if (SBUF_HASOVERFLOWED(s))
+		error = ENOMEM;
 	s->s_buf[s->s_len] = '\0';
 	SBUF_CLEARFLAG(s, SBUF_OVERFLOWED);
 	SBUF_SETFLAG(s, SBUF_FINISHED);
+#ifdef _KERNEL
+	return (error);
+#else
+	/*XXX*/if (error) {
+		errno = error;
+		return (-1);
+	} else
+		return (0);
+#endif
 }
 
 /*
@@ -552,6 +707,8 @@ sbuf_data(struct sbuf *s)
 
 	assert_sbuf_integrity(s);
 	assert_sbuf_state(s, SBUF_FINISHED);
+	KASSERT(s->s_drain == NULL,
+	    ("%s makes no sense on sbuf %p with drain", __func__, s));
 
 	return (s->s_buf);
 }
@@ -565,6 +722,8 @@ sbuf_len(struct sbuf *s)
 
 	assert_sbuf_integrity(s);
 	/* don't care if it's finished or not */
+	KASSERT(s->s_drain == NULL,
+	    ("%s makes no sense on sbuf %p with drain", __func__, s));
 
 	if (SBUF_HASOVERFLOWED(s))
 		return (-1);
@@ -584,6 +743,8 @@ sbuf_delete(struct sbuf *s)
 
 	if (SBUF_ISDYNAMIC(s))
 		SBFREE(s->s_buf);
+	if (s->s_drain != NULL)
+		SBFREE(s->s_drain);
 	isdyn = SBUF_ISDYNSTRUCT(s);
 	bzero(s, sizeof(*s));
 	if (isdyn)

Modified: stable/8/sys/sys/sbuf.h
==============================================================================
--- stable/8/sys/sys/sbuf.h	Wed Jun  1 17:17:02 2011	(r222573)
+++ stable/8/sys/sys/sbuf.h	Wed Jun  1 17:36:52 2011	(r222574)
@@ -33,12 +33,16 @@
 
 #include <sys/_types.h>
 
+struct sbuf;
+struct sbuf_drain_data;
+typedef int (sbuf_drain_func)(void *, const char *, int);
+
 /*
  * Structure definition
  */
 struct sbuf {
 	char		*s_buf;		/* storage buffer */
-	void		*s_unused;	/* binary compatibility. */
+	struct sbuf_drain *s_drain;	/* drain function and data */
 	int		 s_size;	/* size of storage buffer */
 	int		 s_len;		/* current length of string */
 #define	SBUF_FIXEDLEN	0x00000000	/* fixed length buffer (default) */
@@ -69,9 +73,10 @@ int		 sbuf_printf(struct sbuf *, const c
 int		 sbuf_vprintf(struct sbuf *, const char *, __va_list)
 	__printflike(2, 0);
 int		 sbuf_putc(struct sbuf *, int);
+void		 sbuf_set_drain(struct sbuf *, sbuf_drain_func *, void *);
 int		 sbuf_trim(struct sbuf *);
 int		 sbuf_overflowed(struct sbuf *);
-void		 sbuf_finish(struct sbuf *);
+int		 sbuf_finish(struct sbuf *);
 char		*sbuf_data(struct sbuf *);
 int		 sbuf_len(struct sbuf *);
 int		 sbuf_done(struct sbuf *);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201106011736.p51HaqTu079955>