Author: cem Date: Tue May 7 17:47:20 2019 New Revision: 347229 URL: https://svnweb.freebsd.org/changeset/base/347229
Log: device_printf: Use sbuf for more coherent prints on SMP device_printf does multiple calls to printf allowing other console messages to be inserted between the device name, and the rest of the message. This change uses sbuf to compose to two into a single buffer, and prints it all at once. It exposes an sbuf drain function (drain-to-printf) for common use. Update documentation to match; some unit tests included. Submitted by: jmg Sponsored by: Dell EMC Isilon Differential Revision: https://reviews.freebsd.org/D16690 Modified: head/lib/libsbuf/Symbol.map head/lib/libsbuf/tests/sbuf_core_test.c head/lib/libsbuf/tests/sbuf_stdio_test.c head/share/man/man9/Makefile head/share/man/man9/sbuf.9 head/sys/kern/kern_sysctl.c head/sys/kern/subr_bus.c head/sys/kern/subr_prf.c head/sys/sys/sbuf.h Modified: head/lib/libsbuf/Symbol.map ============================================================================== --- head/lib/libsbuf/Symbol.map Tue May 7 16:17:33 2019 (r347228) +++ head/lib/libsbuf/Symbol.map Tue May 7 17:47:20 2019 (r347229) @@ -37,5 +37,6 @@ FBSD_1.4 { FBSD_1.5 { sbuf_putbuf; + sbuf_printf_drain; }; Modified: head/lib/libsbuf/tests/sbuf_core_test.c ============================================================================== --- head/lib/libsbuf/tests/sbuf_core_test.c Tue May 7 16:17:33 2019 (r347228) +++ head/lib/libsbuf/tests/sbuf_core_test.c Tue May 7 17:47:20 2019 (r347229) @@ -63,6 +63,9 @@ ATF_TC_BODY(sbuf_clear_test, tc) */ child_proc = atf_utils_fork(); if (child_proc == 0) { + ATF_REQUIRE_EQ_MSG(0, sbuf_finish(sb), "sbuf_finish failed: %s", + strerror(errno)); + sbuf_putbuf(sb); exit(0); } @@ -100,6 +103,34 @@ ATF_TC_BODY(sbuf_done_and_sbuf_finish_test, tc) sbuf_delete(sb); } +static int +drain_ret0(void *arg, const char *data, int len) +{ + + (void)arg; + (void)data; + (void)len; + + return (0); +} + +ATF_TC_WITHOUT_HEAD(sbuf_drain_ret0_test); +ATF_TC_BODY(sbuf_drain_ret0_test, tc) +{ + struct sbuf *sb; + + sb = sbuf_new_auto(); + + sbuf_set_drain(sb, drain_ret0, NULL); + + sbuf_cat(sb, test_string); + + ATF_CHECK_EQ_MSG(-1, sbuf_finish(sb), + "required to return error when drain func returns 0"); + ATF_CHECK_EQ_MSG(EDEADLK, errno, + "errno required to be EDEADLK when drain func returns 0"); +} + ATF_TC_WITHOUT_HEAD(sbuf_len_test); ATF_TC_BODY(sbuf_len_test, tc) { @@ -131,6 +162,34 @@ ATF_TC_BODY(sbuf_len_test, tc) sbuf_delete(sb); } +ATF_TC_WITHOUT_HEAD(sbuf_new_fixedlen); +ATF_TC_BODY(sbuf_new_fixedlen, tc) +{ + char buf[strlen(test_string) + 1]; + struct sbuf sb; + pid_t child_proc; + + sbuf_new(&sb, buf, sizeof(buf), SBUF_FIXEDLEN); + + sbuf_cat(&sb, test_string); + + child_proc = atf_utils_fork(); + if (child_proc == 0) { + ATF_REQUIRE_EQ_MSG(0, sbuf_finish(&sb), "sbuf_finish failed: %s", + strerror(errno)); + + sbuf_putbuf(&sb); + exit(0); + } + atf_utils_wait(child_proc, 0, test_string, ""); + + sbuf_putc(&sb, ' '); + + ATF_CHECK_EQ_MSG(-1, sbuf_finish(&sb), "failed to return error on overflow"); + + sbuf_delete(&sb); +} + ATF_TC_WITHOUT_HEAD(sbuf_setpos_test); ATF_TC_BODY(sbuf_setpos_test, tc) { @@ -190,7 +249,9 @@ ATF_TP_ADD_TCS(tp) ATF_TP_ADD_TC(tp, sbuf_clear_test); ATF_TP_ADD_TC(tp, sbuf_done_and_sbuf_finish_test); + ATF_TP_ADD_TC(tp, sbuf_drain_ret0_test); ATF_TP_ADD_TC(tp, sbuf_len_test); + ATF_TP_ADD_TC(tp, sbuf_new_fixedlen); #if 0 /* TODO */ #ifdef HAVE_SBUF_CLEAR_FLAGS Modified: head/lib/libsbuf/tests/sbuf_stdio_test.c ============================================================================== --- head/lib/libsbuf/tests/sbuf_stdio_test.c Tue May 7 16:17:33 2019 (r347228) +++ head/lib/libsbuf/tests/sbuf_stdio_test.c Tue May 7 17:47:20 2019 (r347229) @@ -59,6 +59,60 @@ sbuf_vprintf_helper(struct sbuf *sb, const char * rest return (rc); } +ATF_TC_WITHOUT_HEAD(sbuf_printf_drain_null_test); +ATF_TC_BODY(sbuf_printf_drain_null_test, tc) +{ + struct sbuf *sb; + char buf[2]; + pid_t child_proc; + + sb = sbuf_new(NULL, buf, sizeof(buf), SBUF_FIXEDLEN); + ATF_REQUIRE_MSG(sb != NULL, "sbuf_new_auto failed: %s", + strerror(errno)); + + child_proc = atf_utils_fork(); + if (child_proc == 0) { + sbuf_set_drain(sb, sbuf_printf_drain, NULL); + + ATF_REQUIRE_EQ_MSG(0, sbuf_cat(sb, test_string), + "sbuf_cat failed"); + + ATF_CHECK_EQ(0, sbuf_finish(sb)); + exit(0); + } + atf_utils_wait(child_proc, 0, test_string, ""); + + sbuf_delete(sb); +} + +ATF_TC_WITHOUT_HEAD(sbuf_printf_drain_test); +ATF_TC_BODY(sbuf_printf_drain_test, tc) +{ + struct sbuf *sb; + char buf[2]; + pid_t child_proc; + size_t cnt = 0; + + sb = sbuf_new(NULL, buf, sizeof(buf), SBUF_FIXEDLEN); + ATF_REQUIRE_MSG(sb != NULL, "sbuf_new_auto failed: %s", + strerror(errno)); + + child_proc = atf_utils_fork(); + if (child_proc == 0) { + sbuf_set_drain(sb, sbuf_printf_drain, &cnt); + + ATF_REQUIRE_EQ_MSG(0, sbuf_cat(sb, test_string), + "sbuf_cat failed"); + + ATF_CHECK_EQ(0, sbuf_finish(sb)); + ATF_CHECK_EQ(strlen(test_string), cnt); + exit(0); + } + atf_utils_wait(child_proc, 0, test_string, ""); + + sbuf_delete(sb); +} + ATF_TC_WITHOUT_HEAD(sbuf_printf_test); ATF_TC_BODY(sbuf_printf_test, tc) { @@ -106,6 +160,7 @@ ATF_TC_BODY(sbuf_putbuf_test, tc) child_proc = atf_utils_fork(); if (child_proc == 0) { + ATF_CHECK_EQ(0, sbuf_finish(sb)); sbuf_putbuf(sb); exit(0); } @@ -152,6 +207,8 @@ ATF_TC_BODY(sbuf_vprintf_test, tc) ATF_TP_ADD_TCS(tp) { + ATF_TP_ADD_TC(tp, sbuf_printf_drain_null_test); + ATF_TP_ADD_TC(tp, sbuf_printf_drain_test); ATF_TP_ADD_TC(tp, sbuf_printf_test); ATF_TP_ADD_TC(tp, sbuf_putbuf_test); ATF_TP_ADD_TC(tp, sbuf_vprintf_test); Modified: head/share/man/man9/Makefile ============================================================================== --- head/share/man/man9/Makefile Tue May 7 16:17:33 2019 (r347228) +++ head/share/man/man9/Makefile Tue May 7 17:47:20 2019 (r347229) @@ -1780,6 +1780,8 @@ MLINKS+=sbuf.9 sbuf_bcat.9 \ sbuf.9 sbuf_new_auto.9 \ sbuf.9 sbuf_new_for_sysctl.9 \ sbuf.9 sbuf_printf.9 \ + sbuf.9 sbuf_printf_drain.9 \ + sbuf.9 sbuf_putbuf.9 \ sbuf.9 sbuf_putc.9 \ sbuf.9 sbuf_set_drain.9 \ sbuf.9 sbuf_set_flags.9 \ Modified: head/share/man/man9/sbuf.9 ============================================================================== --- head/share/man/man9/sbuf.9 Tue May 7 16:17:33 2019 (r347228) +++ head/share/man/man9/sbuf.9 Tue May 7 17:47:20 2019 (r347229) @@ -25,7 +25,7 @@ .\" .\" $FreeBSD$ .\" -.Dd May 23, 2018 +.Dd May 7, 2019 .Dt SBUF 9 .Os .Sh NAME @@ -58,6 +58,7 @@ .Nm sbuf_start_section , .Nm sbuf_end_section , .Nm sbuf_hexdump , +.Nm sbuf_printf_drain , .Nm sbuf_putbuf .Nd safe string composition .Sh SYNOPSIS @@ -191,6 +192,12 @@ .Fa "const char *hdr" .Fa "int flags" .Fc +.Ft int +.Fo sbuf_printf_drain +.Fa "void *arg" +.Fa "const char *data" +.Fa "int len" +.Fc .Ft void .Fo sbuf_putbuf .Fa "struct sbuf *s" @@ -278,7 +285,9 @@ as a record boundary marker that will be used during d records being split. If a record grows sufficiently large such that it fills the .Fa sbuf -and therefore cannot be drained without being split, an error of EDEADLK is set. +and therefore cannot be drained without being split, an error of +.Er EDEADLK +is set. .El .Pp Note that if @@ -419,7 +428,9 @@ 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. +If the returned drained length is 0, an error of +.Er EDEADLK +is set. 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 @@ -492,7 +503,9 @@ The .Fn sbuf_error function returns any error value that the .Fa sbuf -may have accumulated, either from the drain function, or ENOMEM if the +may have accumulated, either from the drain function, or +.Er ENOMEM +if the .Fa sbuf overflowed. This function is generally not needed and instead the error code from @@ -574,9 +587,29 @@ See the man page for more details on the interface. .Pp The +.Fn sbuf_printf_drain +function is a drain function that will call printf, or log to the console. +The argument +.Fa arg +must be either +.Dv NULL , +or a valid pointer to a +.Vt size_t . +If +.Fa arg +is not +.Dv NULL , +the total bytes drained will be added to the value pointed to by +.Fa arg . +.Pp +The .Fn sbuf_putbuf function printfs the sbuf to stdout if in userland, and to the console and log if in the kernel. +The +.Fa sbuf +must be finished before calling +.Fn sbuf_putbuf . It does not drain the buffer or update any pointers. .Sh NOTES If an operation caused an @@ -655,8 +688,9 @@ function returns the section length or \-1 if the buff .Pp The .Fn sbuf_finish 9 -function (the kernel version) returns ENOMEM if the sbuf overflowed before -being finished, +function (the kernel version) returns +.Er ENOMEM +if the sbuf overflowed before being finished, or returns the error code from the drain if one is attached. .Pp The Modified: head/sys/kern/kern_sysctl.c ============================================================================== --- head/sys/kern/kern_sysctl.c Tue May 7 16:17:33 2019 (r347228) +++ head/sys/kern/kern_sysctl.c Tue May 7 17:47:20 2019 (r347229) @@ -322,13 +322,6 @@ sysctl_load_tunable_by_oid_locked(struct sysctl_oid *o freeenv(penv); } -static int -sbuf_printf_drain(void *arg __unused, const char *data, int len) -{ - - return (printf("%.*s", len, data)); -} - /* * Locate the path to a given oid. Returns the length of the resulting path, * or -1 if the oid was not found. nodes must have room for CTL_MAXNAME Modified: head/sys/kern/subr_bus.c ============================================================================== --- head/sys/kern/subr_bus.c Tue May 7 16:17:33 2019 (r347228) +++ head/sys/kern/subr_bus.c Tue May 7 17:47:20 2019 (r347229) @@ -2431,13 +2431,31 @@ device_print_prettyname(device_t dev) int device_printf(device_t dev, const char * fmt, ...) { + char buf[128]; + struct sbuf sb; + const char *name; va_list ap; - int retval; + size_t retval; - retval = device_print_prettyname(dev); + retval = 0; + + sbuf_new(&sb, buf, sizeof(buf), SBUF_FIXEDLEN); + sbuf_set_drain(&sb, sbuf_printf_drain, &retval); + + name = device_get_name(dev); + + if (name == NULL) + sbuf_cat(&sb, "unknown: "); + else + sbuf_printf(&sb, "%s%d: ", name, device_get_unit(dev)); + va_start(ap, fmt); - retval += vprintf(fmt, ap); + sbuf_vprintf(&sb, fmt, ap); va_end(ap); + + sbuf_finish(&sb); + sbuf_delete(&sb); + return (retval); } Modified: head/sys/kern/subr_prf.c ============================================================================== --- head/sys/kern/subr_prf.c Tue May 7 16:17:33 2019 (r347228) +++ head/sys/kern/subr_prf.c Tue May 7 17:47:20 2019 (r347229) @@ -62,6 +62,8 @@ __FBSDID("$FreeBSD$"); #include <sys/syslog.h> #include <sys/cons.h> #include <sys/uio.h> +#else /* !_KERNEL */ +#include <errno.h> #endif #include <sys/ctype.h> #include <sys/sbuf.h> @@ -1254,3 +1256,42 @@ sbuf_putbuf(struct sbuf *sb) printf("%s", sbuf_data(sb)); } #endif + +int +sbuf_printf_drain(void *arg, const char *data, int len) +{ + size_t *retvalptr; + int r; +#ifdef _KERNEL + char *dataptr; + char oldchr; + + /* + * This is allowed as an extra byte is always resvered for + * terminating NUL byte. Save and restore the byte because + * we might be flushing a record, and there may be valid + * data after the buffer. + */ + oldchr = data[len]; + dataptr = __DECONST(char *, data); + dataptr[len] = '\0'; + + prf_putbuf(dataptr, TOLOG | TOCONS, -1); + r = len; + + dataptr[len] = oldchr; + +#else /* !_KERNEL */ + + r = printf("%.*s", len, data); + if (r < 0) + return (-errno); + +#endif + + retvalptr = arg; + if (retvalptr != NULL) + *retvalptr += r; + + return (r); +} Modified: head/sys/sys/sbuf.h ============================================================================== --- head/sys/sys/sbuf.h Tue May 7 16:17:33 2019 (r347228) +++ head/sys/sys/sbuf.h Tue May 7 17:47:20 2019 (r347229) @@ -103,6 +103,7 @@ void sbuf_start_section(struct sbuf *, ssize_t *); ssize_t sbuf_end_section(struct sbuf *, ssize_t, size_t, int); void sbuf_hexdump(struct sbuf *, const void *, int, const char *, int); +int sbuf_printf_drain(void *arg, const char *data, int len); void sbuf_putbuf(struct sbuf *); #ifdef _KERNEL _______________________________________________ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"