diff mbox series

[v4,08/34] lib/printbuf: Tabstops, indenting

Message ID 20220620004233.3805-9-kent.overstreet@gmail.com (mailing list archive)
State New
Headers show
Series Printbufs - new data structure for building strings | expand

Commit Message

Kent Overstreet June 20, 2022, 12:42 a.m. UTC
This patch adds two new features to printbuf for structured formatting:

 - Indent level: the indent level, as a number of spaces, may be
   increased with pr_indent_add() and decreased with pr_indent_sub().

   Subsequent lines, when started with pr_newline() (not "\n", although
   that may change) will then be intended according to the current
   indent level. This helps with pretty-printers that structure a large
   amonut of data across multiple lines and multiple functions.

 - Tabstops: Tabstops may be set by assigning to the printbuf->tabstops
   array.

   Then, pr_tab() may be used to advance to the next tabstop, printing
   as many spaces as required - leaving previous output left justified
   to the previous tabstop. pr_tab_rjust() advances to the next tabstop
   but inserts the spaces just after the previous tabstop - right
   justifying the previously-outputted text to the next tabstop.

Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com>
---
 include/linux/printbuf.h |  30 ++++++++++
 lib/printbuf.c           | 125 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 155 insertions(+)

Comments

Rasmus Villemoes June 21, 2022, 8:14 a.m. UTC | #1
On 20/06/2022 02.42, Kent Overstreet wrote:
> This patch adds two new features to printbuf for structured formatting:
> 
>  - Indent level: the indent level, as a number of spaces, may be
>    increased with pr_indent_add() and decreased with pr_indent_sub().
> 
>    Subsequent lines, when started with pr_newline() (not "\n", although
>    that may change) will then be intended according to the current
>    indent level. This helps with pretty-printers that structure a large
>    amonut of data across multiple lines and multiple functions.
> 
>  - Tabstops: Tabstops may be set by assigning to the printbuf->tabstops
>    array.
> 
>    Then, pr_tab() may be used to advance to the next tabstop, printing
>    as many spaces as required - leaving previous output left justified
>    to the previous tabstop. pr_tab_rjust() advances to the next tabstop
>    but inserts the spaces just after the previous tabstop - right
>    justifying the previously-outputted text to the next tabstop.

I am really, really, really not convinced that we want or need this. But
as long as this doesn't add overhead to those not using it (in
particular, as long as it doesn't grow a "scan whatever contents was
just added for maybe a \n so ->last_newline can be updated"), meh.

> + *
> + * Make sure you use prt_newline() instead of \n in the format string for indent
> + * level and tabstops to work corretly.
>   */
>  
>  #include <linux/kernel.h>
> @@ -45,18 +62,29 @@ struct printbuf {
>  	char			*buf;
>  	unsigned		size;
>  	unsigned		pos;
> +	unsigned		last_newline;
> +	unsigned		last_field;
> +	unsigned		indent;
>  	/*
>  	 * If nonzero, allocations will be done with GFP_ATOMIC:
>  	 */
>  	u8			atomic;
>  	bool			allocation_failure:1;
>  	bool			heap_allocated:1;
> +	u8			tabstop;
> +	u8			tabstops[4];
>  };
>  
>  int printbuf_make_room(struct printbuf *, unsigned);
>  const char *printbuf_str(const struct printbuf *);
>  void printbuf_exit(struct printbuf *);
>  
> +void prt_newline(struct printbuf *);
> +void printbuf_indent_add(struct printbuf *, unsigned);
> +void printbuf_indent_sub(struct printbuf *, unsigned);
> +void prt_tab(struct printbuf *);
> +void prt_tab_rjust(struct printbuf *);
> +
>  /* Initializer for a heap allocated printbuf: */
>  #define PRINTBUF ((struct printbuf) { .heap_allocated = true })
>  
> @@ -187,6 +215,8 @@ static inline void printbuf_reset(struct printbuf *buf)
>  {
>  	buf->pos		= 0;
>  	buf->allocation_failure	= 0;
> +	buf->indent		= 0;
> +	buf->tabstop		= 0;
>  }
>  
>  /**
> diff --git a/lib/printbuf.c b/lib/printbuf.c
> index 8c70128e31..a7f80f63ca 100644
> --- a/lib/printbuf.c
> +++ b/lib/printbuf.c
> @@ -12,6 +12,11 @@
>  #include <linux/slab.h>
>  #include <linux/printbuf.h>
>  
> +static inline size_t printbuf_linelen(struct printbuf *buf)
> +{
> +	return buf->pos - buf->last_newline;
> +}
> +
>  int printbuf_make_room(struct printbuf *out, unsigned extra)
>  {
>  	unsigned new_size;
> @@ -69,3 +74,123 @@ void printbuf_exit(struct printbuf *buf)
>  	}
>  }
>  EXPORT_SYMBOL(printbuf_exit);
> +
> +void prt_newline(struct printbuf *buf)
> +{
> +	unsigned i;
> +
> +	printbuf_make_room(buf, 1 + buf->indent);
> +
> +	__prt_char(buf, '\n');
> +
> +	buf->last_newline	= buf->pos;
> +
> +	for (i = 0; i < buf->indent; i++)
> +		__prt_char(buf, ' ');

Why the loop? Don't you have a _chars variant?

> +void printbuf_indent_add(struct printbuf *buf, unsigned spaces)
> +{
> +	if (WARN_ON_ONCE(buf->indent + spaces < buf->indent))
> +		spaces = 0;
> +
> +	buf->indent += spaces;
> +	while (spaces--)
> +		prt_char(buf, ' ');
> +}
> +EXPORT_SYMBOL(printbuf_indent_add);
> +
> +/**
> + * printbuf_indent_sub - subtract from the current indent level
> + *
> + * @buf: printbuf to control
> + * @spaces: number of spaces to subtract from the current indent level
> + *
> + * Subsequent lines, and the current line if the output position is at the start
> + * of the current line, will be indented by @spaces less spaces.
> + */
> +void printbuf_indent_sub(struct printbuf *buf, unsigned spaces)
> +{
> +	if (WARN_ON_ONCE(spaces > buf->indent))
> +		spaces = buf->indent;
> +
> +	if (buf->last_newline + buf->indent == buf->pos) {
> +		buf->pos -= spaces;
> +		printbuf_nul_terminate(buf);
> +	}
> +	buf->indent -= spaces;
> +}
> +EXPORT_SYMBOL(printbuf_indent_sub);
> +
> +/**
> + * prt_tab - Advance printbuf to the next tabstop
> + *
> + * @buf: printbuf to control
> + *
> + * Advance output to the next tabstop by printing spaces.
> + */
> +void prt_tab(struct printbuf *out)
> +{
> +	int spaces = max_t(int, 0, out->tabstops[out->tabstop] - printbuf_linelen(out));
> +
> +	BUG_ON(out->tabstop > ARRAY_SIZE(out->tabstops));

So this accesses out->tabstops first, then does a (buggy) bounds check.
And kills the machine if somebody managed to corrupt ->tabstop. Instead
of limping along and living with less-pretty-printed output.

I don't think you'll get this code accepted by the Great Penguin.

Rasmus
diff mbox series

Patch

diff --git a/include/linux/printbuf.h b/include/linux/printbuf.h
index 382863afa7..1e43c47891 100644
--- a/include/linux/printbuf.h
+++ b/include/linux/printbuf.h
@@ -36,6 +36,23 @@ 
  * memory allocation failure we usually don't want to bail out and unwind - we
  * want to print what we've got, on a best-effort basis. But code that does want
  * to return -ENOMEM may check printbuf.allocation_failure.
+ *
+ * Indenting, tabstops:
+ *
+ * To aid is writing multi-line pretty printers spread across multiple
+ * functions, printbufs track the current indent level.
+ *
+ * printbuf_indent_push() and printbuf_indent_pop() increase and decrease the current indent
+ * level, respectively.
+ *
+ * To use tabstops, set printbuf->tabstops[]; they are in units of spaces, from
+ * start of line. Once set, prt_tab() will output spaces up to the next tabstop.
+ * prt_tab_rjust() will also advance the current line of text up to the next
+ * tabstop, but it does so by shifting text since the previous tabstop up to the
+ * next tabstop - right justifying it.
+ *
+ * Make sure you use prt_newline() instead of \n in the format string for indent
+ * level and tabstops to work corretly.
  */
 
 #include <linux/kernel.h>
@@ -45,18 +62,29 @@  struct printbuf {
 	char			*buf;
 	unsigned		size;
 	unsigned		pos;
+	unsigned		last_newline;
+	unsigned		last_field;
+	unsigned		indent;
 	/*
 	 * If nonzero, allocations will be done with GFP_ATOMIC:
 	 */
 	u8			atomic;
 	bool			allocation_failure:1;
 	bool			heap_allocated:1;
+	u8			tabstop;
+	u8			tabstops[4];
 };
 
 int printbuf_make_room(struct printbuf *, unsigned);
 const char *printbuf_str(const struct printbuf *);
 void printbuf_exit(struct printbuf *);
 
+void prt_newline(struct printbuf *);
+void printbuf_indent_add(struct printbuf *, unsigned);
+void printbuf_indent_sub(struct printbuf *, unsigned);
+void prt_tab(struct printbuf *);
+void prt_tab_rjust(struct printbuf *);
+
 /* Initializer for a heap allocated printbuf: */
 #define PRINTBUF ((struct printbuf) { .heap_allocated = true })
 
@@ -187,6 +215,8 @@  static inline void printbuf_reset(struct printbuf *buf)
 {
 	buf->pos		= 0;
 	buf->allocation_failure	= 0;
+	buf->indent		= 0;
+	buf->tabstop		= 0;
 }
 
 /**
diff --git a/lib/printbuf.c b/lib/printbuf.c
index 8c70128e31..a7f80f63ca 100644
--- a/lib/printbuf.c
+++ b/lib/printbuf.c
@@ -12,6 +12,11 @@ 
 #include <linux/slab.h>
 #include <linux/printbuf.h>
 
+static inline size_t printbuf_linelen(struct printbuf *buf)
+{
+	return buf->pos - buf->last_newline;
+}
+
 int printbuf_make_room(struct printbuf *out, unsigned extra)
 {
 	unsigned new_size;
@@ -69,3 +74,123 @@  void printbuf_exit(struct printbuf *buf)
 	}
 }
 EXPORT_SYMBOL(printbuf_exit);
+
+void prt_newline(struct printbuf *buf)
+{
+	unsigned i;
+
+	printbuf_make_room(buf, 1 + buf->indent);
+
+	__prt_char(buf, '\n');
+
+	buf->last_newline	= buf->pos;
+
+	for (i = 0; i < buf->indent; i++)
+		__prt_char(buf, ' ');
+
+	printbuf_nul_terminate(buf);
+
+	buf->last_field		= buf->pos;
+	buf->tabstop = 0;
+}
+EXPORT_SYMBOL(prt_newline);
+
+/**
+ * printbuf_indent_add - add to the current indent level
+ *
+ * @buf: printbuf to control
+ * @spaces: number of spaces to add to the current indent level
+ *
+ * Subsequent lines, and the current line if the output position is at the start
+ * of the current line, will be indented by @spaces more spaces.
+ */
+void printbuf_indent_add(struct printbuf *buf, unsigned spaces)
+{
+	if (WARN_ON_ONCE(buf->indent + spaces < buf->indent))
+		spaces = 0;
+
+	buf->indent += spaces;
+	while (spaces--)
+		prt_char(buf, ' ');
+}
+EXPORT_SYMBOL(printbuf_indent_add);
+
+/**
+ * printbuf_indent_sub - subtract from the current indent level
+ *
+ * @buf: printbuf to control
+ * @spaces: number of spaces to subtract from the current indent level
+ *
+ * Subsequent lines, and the current line if the output position is at the start
+ * of the current line, will be indented by @spaces less spaces.
+ */
+void printbuf_indent_sub(struct printbuf *buf, unsigned spaces)
+{
+	if (WARN_ON_ONCE(spaces > buf->indent))
+		spaces = buf->indent;
+
+	if (buf->last_newline + buf->indent == buf->pos) {
+		buf->pos -= spaces;
+		printbuf_nul_terminate(buf);
+	}
+	buf->indent -= spaces;
+}
+EXPORT_SYMBOL(printbuf_indent_sub);
+
+/**
+ * prt_tab - Advance printbuf to the next tabstop
+ *
+ * @buf: printbuf to control
+ *
+ * Advance output to the next tabstop by printing spaces.
+ */
+void prt_tab(struct printbuf *out)
+{
+	int spaces = max_t(int, 0, out->tabstops[out->tabstop] - printbuf_linelen(out));
+
+	BUG_ON(out->tabstop > ARRAY_SIZE(out->tabstops));
+
+	prt_chars(out, ' ', spaces);
+
+	out->last_field = out->pos;
+	out->tabstop++;
+}
+EXPORT_SYMBOL(prt_tab);
+
+/**
+ * prt_tab_rjust - Advance printbuf to the next tabstop, right justifying
+ * previous output
+ *
+ * @buf: printbuf to control
+ *
+ * Advance output to the next tabstop by inserting spaces immediately after the
+ * previous tabstop, right justifying previously outputted text.
+ */
+void prt_tab_rjust(struct printbuf *buf)
+{
+	BUG_ON(buf->tabstop > ARRAY_SIZE(buf->tabstops));
+
+	if (printbuf_linelen(buf) < buf->tabstops[buf->tabstop]) {
+		unsigned move = buf->pos - buf->last_field;
+		unsigned shift = buf->tabstops[buf->tabstop] -
+			printbuf_linelen(buf);
+
+		printbuf_make_room(buf, shift);
+
+		if (buf->last_field + shift < buf->size)
+			memmove(buf->buf + buf->last_field + shift,
+				buf->buf + buf->last_field,
+				min(move, buf->size - 1 - buf->last_field - shift));
+
+		if (buf->last_field < buf->size)
+			memset(buf->buf + buf->last_field, ' ',
+			       min(shift, buf->size - buf->last_field));
+
+		buf->pos += shift;
+		printbuf_nul_terminate(buf);
+	}
+
+	buf->last_field = buf->pos;
+	buf->tabstop++;
+}
+EXPORT_SYMBOL(prt_tab_rjust);