[PATCH] doc: convert printk-formats.txt to rst

[PATCH] doc: convert printk-formats.txt to rst

[Tobin C. Harding]

Documentation/printk-formats.txt is a candidate for conversion to
ReStructuredText format. Some effort has already been made to do this
conversion even thought the suffix is currently .txt

Changes required to complete conversion

- Add double backticks where needed.
- Add entry to Documentation/index.rst
- Use flat-table instead of ASCII table.
- Fix minor grammatical errors.
- Capitalize headers and correctly order heading adornments.
- Use 'Passed by reference' uniformly.
- Update pointer documentation around %px specifier.
- Fix erroneous double backticks (to commas).
- Simplify documentation for kobject.
- Convert lib/vsnprintf.c function docs to use kernel-docs and
  include in Documentation/printk-formats.rst

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---

The last two need special reviewing please. In particular the conversion
of kernel-docs in vsnprintf.c attempt was made to reduce documentation
duplication with comments in the source code being simplified in order
to be suitable for inclusion in Documentation/printk-formats.rst

I used -M when formatting the patch. If you don't like the diff with
this flag just holla.

thanks,
Tobin.

 Documentation/index.rst                            |  10 +
 .../{printk-formats.txt => printk-formats.rst}     | 295 ++++++++++++---------
 lib/vsprintf.c                                     | 160 +++++------
 3 files changed, 235 insertions(+), 230 deletions(-)
 rename Documentation/{printk-formats.txt => printk-formats.rst} (61%)

diff --git a/Documentation/index.rst b/Documentation/index.rst
index cb7f1ba5b3b1..83ace60efbe7 100644
--- a/Documentation/index.rst
+++ b/Documentation/index.rst
@@ -87,6 +87,16 @@ implementation.
 
    sh/index
 
+Miscellaneous documentation
+---------------------------
+
+These guides contain general information useful when writing kernel code.
+
+.. toctree::
+   :maxdepth: 1
+
+   printk-formats
+
 Korean translations
 -------------------
 
diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.rst
similarity index 61%
rename from Documentation/printk-formats.txt
rename to Documentation/printk-formats.rst
index aa0a776c817a..51449d213748 100644
--- a/Documentation/printk-formats.txt
+++ b/Documentation/printk-formats.rst
@@ -1,6 +1,6 @@
-=========================================
-How to get printk format specifiers right
-=========================================
+=============================================
+How to Get ``printk`` Format Specifiers Right
+=============================================
 
 :Author: Randy Dunlap <rdunlap@infradead.org>
 :Author: Andrew Murray <amurray@mpc-data.co.uk>
@@ -8,56 +8,91 @@ How to get printk format specifiers right
 Integer types
 =============
 
-::
+For printing integer types, we have the following format specifiers:
+		
+   .. flat-table:: 
+      :widths: 2 2
+
+      * - **Type**
+	- **Specifier**
+
+      * - ``int``
+        - ``%d`` or ``%x``
+
+      * - ``unsigned int``
+	- ``%u`` or ``%x``
+
+      * - ``long``
+	- ``%ld`` or ``%lx``
+
+      * - ``unsigned long``
+	- ``%lu`` or ``%lx``
+
+      * - ``long long``
+	- ``%lld`` or ``%llx``
 
-	If variable is of Type,		use printk format specifier:
-	------------------------------------------------------------
-		int			%d or %x
-		unsigned int		%u or %x
-		long			%ld or %lx
-		unsigned long		%lu or %lx
-		long long		%lld or %llx
-		unsigned long long	%llu or %llx
-		size_t			%zu or %zx
-		ssize_t			%zd or %zx
-		s32			%d or %x
-		u32			%u or %x
-		s64			%lld or %llx
-		u64			%llu or %llx
-
-If <type> is dependent on a config option for its size (e.g., ``sector_t``,
+      * - ``unsigned long long``
+	- ``%llu`` or ``%llx``
+
+      * - ``size_t``
+	- ``%zu`` or ``%zx``
+
+      * - ``ssize_t``
+	- ``%zd`` or ``%zx``
+
+      * - ``s32``
+	- ``%d`` or ``%x``
+
+      * - ``u32``
+	- ``%u`` or ``%x``
+
+      * - ``s64``
+	- ``%lld`` or ``%llx``
+
+      * - ``u64``
+	- ``%llu`` or ``%llx``
+
+
+If ``<type>`` is dependent on a config option for its size (e.g., ``sector_t``,
 ``blkcnt_t``) or is architecture-dependent for its size (e.g., ``tcflag_t``),
 use a format specifier of its largest possible type and explicitly cast to it.
 
 Example::
 
-	printk("test: sector number/total blocks: %llu/%llu\n",
-		(unsigned long long)sector, (unsigned long long)blockcount);
+	printk("test: total blocks: %llu\n", (unsigned long long)blockcount);
 
-Reminder: ``sizeof()`` result is of type ``size_t``.
+Reminder: ``sizeof()`` returns type ``size_t``.
 
-The kernel's printf does not support ``%n``. For obvious reasons, floating
+The kernel's printf does not support ``%n``. For obvious reasons floating
 point formats (``%e, %f, %g, %a``) are also not recognized. Use of any
 unsupported specifier or length qualifier results in a WARN and early
-return from vsnprintf.
-
-Raw pointer value SHOULD be printed with %p. The kernel supports
-the following extended format specifiers for pointer types:
+return from ``vsnprintf()``.
 
 Pointer Types
 =============
 
-Pointers printed without a specifier extension (i.e unadorned %p) are
-hashed to give a unique identifier without leaking kernel addresses to user
-space. On 64 bit machines the first 32 bits are zeroed. If you _really_
-want the address see %px below.
+A raw pointer value may be printed with ``%p`` which will hash the address
+before printing. The Kernel also supports extended specifiers for printing
+pointers of different types.
+
+.. kernel-doc:: lib/vsprintf.c
+     :doc: Extended Format Pointer Specifiers
+
+
+Plain Pointers
+--------------
 
 ::
 
 	%p	abcdef12 or 00000000abcdef12
 
+Pointers printed without a specifier extension (i.e unadorned ``%p``) are
+hashed to give a unique identifier without leaking kernel addresses to user
+space. On 64 bit machines the first 32 bits are zeroed. If you *really*
+want the address see ``%px`` below.
+
 Symbols/Function Pointers
-=========================
+-------------------------
 
 ::
 
@@ -69,61 +104,60 @@ Symbols/Function Pointers
 	%ps	versatile_init
 	%pB	prev_fn_of_versatile_init+0x88/0x88
 
-The ``F`` and ``f`` specifiers are for printing function pointers,
-for example, f->func, &gettimeofday. They have the same result as
-``S`` and ``s`` specifiers. But they do an extra conversion on
-ia64, ppc64 and parisc64 architectures where the function pointers
-are actually function descriptors.
+The ``F`` and ``f`` specifiers are for printing function pointers, for
+example, ``f->func``, ``&gettimeofday``. They have the same result as ``S``
+and ``s`` specifiers. But they do an extra conversion on ia64, ppc64 and
+parisc64 architectures where the function pointers are actually function
+descriptors.
 
 The ``S`` and ``s`` specifiers can be used for printing symbols
-from direct addresses, for example, __builtin_return_address(0),
-(void *)regs->ip. They result in the symbol name with (``S``) or
+from direct addresses, for example, ``__builtin_return_address(0)``,
+``(void *)regs->ip``. They result in the symbol name with (``S``) or
 without (``s``) offsets. If KALLSYMS are disabled then the symbol
 address is printed instead.
 
 The ``B`` specifier results in the symbol name with offsets and should be
 used when printing stack backtraces. The specifier takes into
 consideration the effect of compiler optimisations which may occur
-when tail-call``s are used and marked with the noreturn GCC attribute.
+when tail-call's are used and marked with the ``noreturn`` GCC attribute.
 
 Examples::
 
 	printk("Going to call: %pF\n", gettimeofday);
 	printk("Going to call: %pF\n", p->func);
 	printk("%s: called from %pS\n", __func__, (void *)_RET_IP_);
-	printk("%s: called from %pS\n", __func__,
-				(void *)__builtin_return_address(0));
+	printk("%s: called from %pS\n", __func__, (void *)__builtin_return_address(0));
 	printk("Faulted at %pS\n", (void *)regs->ip);
 	printk(" %s%pB\n", (reliable ? "" : "? "), (void *)*stack);
 
 Kernel Pointers
-===============
+---------------
 
 ::
 
 	%pK	01234567 or 0123456789abcdef
 
 For printing kernel pointers which should be hidden from unprivileged
-users. The behaviour of ``%pK`` depends on the ``kptr_restrict sysctl`` - see
-Documentation/sysctl/kernel.txt for more details.
+users. The behaviour of ``%pK`` depends on the ``kptr_restrict`` sysctl -
+see ``Documentation/sysctl/kernel.txt`` for more details.
 
 Unmodified Addresses
-====================
+--------------------
 
 ::
 
 	%px	01234567 or 0123456789abcdef
 
-For printing pointers when you _really_ want to print the address. Please
+For printing pointers when you *really* want to print the address. Please
 consider whether or not you are leaking sensitive information about the
-Kernel layout in memory before printing pointers with %px. %px is
-functionally equivalent to %lx. %px is preferred to %lx because it is more
-uniquely grep'able. If, in the future, we need to modify the way the Kernel
-handles printing pointers it will be nice to be able to find the call
-sites.
+kernel memory layout before printing pointers with ``%px``. ``%px`` is
+functionally equivalent to ``%lx`` (or ``%lu``). ``%px``, however, is
+preferable because it is more uniquely grep'able. If, in the future, we need
+to modify the way the Kernel handles printing pointers we will be better
+equipped to find the call sites.
 
 Struct Resources
-================
+----------------
 
 ::
 
@@ -132,12 +166,13 @@ Struct Resources
 	%pR	[mem 0x60000000-0x6fffffff pref] or
 		[mem 0x0000000060000000-0x000000006fffffff pref]
 
-For printing struct resources. The ``R`` and ``r`` specifiers result in a
+For printing ``struct resources``. The ``R`` and ``r`` specifiers result in a
 printed resource with (``R``) or without (``r``) a decoded flags member.
+
 Passed by reference.
 
-Physical addresses types ``phys_addr_t``
-========================================
+Physical Address Types ``phys_addr_t``
+--------------------------------------
 
 ::
 
@@ -145,20 +180,24 @@ Physical addresses types ``phys_addr_t``
 
 For printing a ``phys_addr_t`` type (and its derivatives, such as
 ``resource_size_t``) which can vary based on build options, regardless of
-the width of the CPU data path. Passed by reference.
+the width of the CPU data path.
+
+Passed by reference.
 
-DMA addresses types ``dma_addr_t``
-==================================
+DMA Address Types ``dma_addr_t``
+--------------------------------
 
 ::
 
 	%pad	0x01234567 or 0x0123456789abcdef
 
 For printing a ``dma_addr_t`` type which can vary based on build options,
-regardless of the width of the CPU data path. Passed by reference.
+regardless of the width of the CPU data path.
 
-Raw buffer as an escaped string
-===============================
+Passed by reference.
+
+Raw Buffer as an Escaped String
+-------------------------------
 
 ::
 
@@ -168,7 +207,7 @@ For printing raw buffer as an escaped string. For the following buffer::
 
 		1b 62 20 5c 43 07 22 90 0d 5d
 
-few examples show how the conversion would be done (the result string
+A few examples show how the conversion would be done (the result string
 without surrounding quotes)::
 
 		%*pE		"\eb \C\a"\220\r]"
@@ -194,8 +233,8 @@ printing SSIDs.
 
 If field width is omitted the 1 byte only will be escaped.
 
-Raw buffer as a hex string
-==========================
+Raw Buffer as a Hex String
+--------------------------
 
 ::
 
@@ -205,11 +244,11 @@ Raw buffer as a hex string
 	%*phN	000102 ... 3f
 
 For printing a small buffers (up to 64 bytes long) as a hex string with
-certain separator. For the larger buffers consider to use
+certain separator. For the larger buffers consider using
 :c:func:`print_hex_dump`.
 
-MAC/FDDI addresses
-==================
+MAC/FDDI Addresses
+------------------
 
 ::
 
@@ -233,8 +272,8 @@ of Bluetooth addresses which are in the little endian order.
 
 Passed by reference.
 
-IPv4 addresses
-==============
+IPv4 Addresses
+--------------
 
 ::
 
@@ -252,8 +291,8 @@ no specifier is provided the default network/big endian order is used.
 
 Passed by reference.
 
-IPv6 addresses
-==============
+IPv6 Addresses
+--------------
 
 ::
 
@@ -271,8 +310,8 @@ http://tools.ietf.org/html/rfc5952
 
 Passed by reference.
 
-IPv4/IPv6 addresses (generic, with port, flowinfo, scope)
-=========================================================
+IPv4/IPv6 Addresses (generic, with port, flowinfo or scope)
+---------------------------------------------------------------
 
 ::
 
@@ -282,8 +321,8 @@ IPv4/IPv6 addresses (generic, with port, flowinfo, scope)
 	%pISpc	1.2.3.4:12345	or [1:2:3:4:5:6:7:8]:12345
 	%p[Ii]S[pfschnbl]
 
-For printing an IP address without the need to distinguish whether it``s
-of type AF_INET or AF_INET6, a pointer to a valid ``struct sockaddr``,
+For printing an IP address without the need to distinguish whether it's
+of type AF_INET or AF_INET6. A pointer to a valid ``struct sockaddr``,
 specified through ``IS`` or ``iS``, can be passed to this format specifier.
 
 The additional ``p``, ``f``, and ``s`` specifiers are used to specify port
@@ -308,8 +347,8 @@ Further examples::
 	%pISsc		1.2.3.4		or [1:2:3:4:5:6:7:8]%1234567890
 	%pISpfc		1.2.3.4:12345	or [1:2:3:4:5:6:7:8]:12345/123456789
 
-UUID/GUID addresses
-===================
+UUID/GUID Addresses
+-------------------
 
 ::
 
@@ -318,18 +357,18 @@ UUID/GUID addresses
 	%pUl	03020100-0504-0706-0809-0a0b0c0e0e0f
 	%pUL	03020100-0504-0706-0809-0A0B0C0E0E0F
 
-For printing 16-byte UUID/GUIDs addresses. The additional 'l', 'L',
-'b' and 'B' specifiers are used to specify a little endian order in
-lower ('l') or upper case ('L') hex characters - and big endian order
-in lower ('b') or upper case ('B') hex characters.
+For printing 16-byte UUID/GUIDs addresses. The additional ``l``, ``L``,
+``b`` and ``B`` specifiers are used to specify a little endian order in
+lower (``l``) or upper case (``L``) hex digits - and big endian order
+in lower (``b``) or upper case (``B``) hex digits.
 
 Where no additional specifiers are used the default big endian
-order with lower case hex characters will be printed.
+order with lower case hex digits will be printed.
 
 Passed by reference.
 
-dentry names
-============
+Dentry Names
+------------
 
 ::
 
@@ -343,24 +382,24 @@ equivalent of ``%s`` ``dentry->d_name.name`` we used to use, ``%pd<n>`` prints
 
 Passed by reference.
 
-block_device names
-==================
+block_device Names
+------------------
 
 ::
 
 	%pg	sda, sda1 or loop0p1
 
-For printing name of block_device pointers.
+For printing name of ``block_device`` pointers.
 
 struct va_format
-================
+----------------
 
 ::
 
 	%pV
 
-For printing struct va_format structures. These contain a format string
-and va_list as follows::
+For printing ``struct va_format`` structures. These contain a format string
+and ``va_list`` as follows::
 
 	struct va_format {
 		const char *fmt;
@@ -370,36 +409,33 @@ and va_list as follows::
 Implements a "recursive vsnprintf".
 
 Do not use this feature without some mechanism to verify the
-correctness of the format string and va_list arguments.
+correctness of the format string and ``va_list`` arguments.
 
 Passed by reference.
 
 kobjects
-========
-
+--------
+	
 ::
 
-	%pO
+	%pOF[fnpPcCF]
 
-	Base specifier for kobject based structs. Must be followed with
-	character for specific type of kobject as listed below:
 
-	Device tree nodes:
+For printing kobject based structs (device nodes). Default behaviour is
+equivalent to ``%pOFf``.
 
-	%pOF[fnpPcCF]
+	- ``f`` device node full_name
+	- ``n`` device node name
+	- ``p`` device node phandle
+	- ``P`` device node path spec (name + @unit)
+	- ``F`` device node flags
+	- ``c`` major compatible string
+	- ``C`` full compatible string
 
-	For printing device tree nodes. The optional arguments are:
-	    f device node full_name
-	    n device node name
-	    p device node phandle
-	    P device node path spec (name + @unit)
-	    F device node flags
-	    c major compatible string
-	    C full compatible string
-	Without any arguments prints full_name (same as %pOFf)
-	The separator when using multiple arguments is ':'
+The separator when using multiple arguments is ``:``
 
-	Examples:
+Examples:
+::
 
 	%pOF	/foo/bar@0			- Node full name
 	%pOFf	/foo/bar@0			- Same as above
@@ -412,11 +448,10 @@ kobjects
 							P - Populated
 							B - Populated bus
 
-	Passed by reference.
-
+Passed by reference.
 
 struct clk
-==========
+----------
 
 ::
 
@@ -424,14 +459,14 @@ struct clk
 	%pCn	pll1
 	%pCr	1560000000
 
-For printing struct clk structures. ``%pC`` and ``%pCn`` print the name
+For printing ``struct clk structures``. ``%pC`` and ``%pCn`` print the name
 (Common Clock Framework) or address (legacy clock framework) of the
 structure; ``%pCr`` prints the current clock rate.
 
 Passed by reference.
 
-bitmap and its derivatives such as cpumask and nodemask
-=======================================================
+Bitmap and its Derivatives (such as cpumask and nodemask)
+---------------------------------------------------------
 
 ::
 
@@ -439,13 +474,13 @@ bitmap and its derivatives such as cpumask and nodemask
 	%*pbl	0,3-6,8-10
 
 For printing bitmap and its derivatives such as cpumask and nodemask,
-``%*pb`` output the bitmap with field width as the number of bits and ``%*pbl``
-output the bitmap as range list with field width as the number of bits.
+``%*pb`` outputs the bitmap with field width as the number of bits and ``%*pbl``
+outputs the bitmap as range list with field width as the number of bits.
 
 Passed by reference.
 
-Flags bitfields such as page flags, gfp_flags
-=============================================
+Flags Bitfields (such as page flags, gfp_flags)
+-----------------------------------------------
 
 ::
 
@@ -459,25 +494,27 @@ character. Currently supported are [p]age flags, [v]ma_flags (both
 expect ``unsigned long *``) and [g]fp_flags (expects ``gfp_t *``). The flag
 names and print order depends on the particular	type.
 
-Note that this format should not be used directly in :c:func:`TP_printk()` part
-of a tracepoint. Instead, use the ``show_*_flags()`` functions from
-<trace/events/mmflags.h>.
+Note that this format should not be used directly in the
+:c:func:`TP_printk()` part of a tracepoint. Instead, use the
+``show_*_flags()`` functions from ``<trace/events/mmflags.h>``. 
 
 Passed by reference.
 
-Network device features
-=======================
+Network Device Features
+-----------------------
 
 ::
 
 	%pNF	0x000000000000c000
 
-For printing netdev_features_t.
+For printing ``netdev_features_t``.
 
 Passed by reference.
 
-If you add other ``%p`` extensions, please extend lib/test_printf.c with
-one or more test cases, if at all feasible.
+Thanks
+======
 
+If you add other ``%p`` extensions, please extend ``lib/test_printf.c``
+with one or more test cases, if at all feasible.
 
 Thank you for your cooperation and attention.
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 01c3957b2de6..f9247b78e8ef 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1727,115 +1727,73 @@ static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec)
 	return number(buf, end, hashval, spec);
 }
 
+/**
+ * DOC: Extended Format Pointer Specifiers
+ *
+ * Briefly we handle the following extensions:
+ *
+ * - ``F`` - For symbolic function descriptor pointers with offset.
+ * - ``f`` - For simple symbolic function names without offset.
+ *
+ * - ``S`` - For symbolic direct pointers with offset.
+ * - ``s`` - For symbolic direct pointers without offset.
+ * - ``[FfSs]R`` - As above with ``__builtin_extract_return_addr()`` translation.
+ * - ``B`` - For backtraced symbolic direct pointers with offset.
+ * - ``R`` - For decoded struct resource, e.g., [mem 0x0-0x1f 64bit pref].
+ * - ``r`` - For raw struct resource, e.g., [mem 0x0-0x1f flags 0x201].
+ * - ``b[l]`` - For a bitmap, the number of bits is determined by the field
+ *   width which must be explicitly specified either as part of the format
+ *   string ``32b[l]`` or through ``*b[l]``, ``[l]`` selects range-list format
+ *   instead of hex format.
+ * - ``M`` - For a 6-byte MAC address, it prints the address in the usual
+ *   colon-separated hex notation.
+ * - ``m`` - For a 6-byte MAC address, it prints the hex address without colons.
+ * - ``MF`` - For a 6-byte MAC FDDI address, it prints the address with a
+ *   dash-separated hex notation.
+ * - ``[mM]R`` - For a 6-byte MAC address, Reverse order (Bluetooth).
+ * - ``I[46]`` - For IPv4/IPv6 addresses printed in the usual way.
+ * - ``I[S][pfs]`` - For generic IPv4/IPv6 address (struct sockaddr *) that falls
+ *   back to ``[4]`` or ``[6]`` and is able to print port ``[p]``,
+ *   flowinfo ``[f]``, scope ``[s]``.
+ * - ``i[46]`` - For 'raw' IPv4/IPv6 addresses IPv6 omits the colons (01020304...0f)
+ *   IPv4 uses dot-separated decimal with leading 0's (010.123.045.006).
+ * - ``i[S][pfs]`` - For generic IPv4/IPv6 address (struct sockaddr *) that falls back
+ *   to ``[4]`` or ``[6]`` (``[pfs]`` as above).
+ * - ``[Ii][4S][hnbl]`` - For IPv4 addresses in host, network, big or little endian order.
+ * - ``I[6S]c`` - For IPv6 addresses printed as per http://tools.ietf.org/html/rfc5952.
+ * - ``E[achnops]`` - For an escaped buffer.
+ * - ``U`` - For a 16 byte UUID/GUID.
+ * - ``V`` - For a ``struct va_format`` which contains a format ``string *``
+ *   and ``va_list *``.
+ * - ``K`` -  For a kernel pointer that should be hidden from unprivileged users.
+ * - ``NF`` - For a ``netdev_features_t``.
+ * - ``h[CDN]`` - For a variable-length buffer.
+ * - ``a[pd]`` - For address types ``[p] phys_addr_t``, ``[d] dma_addr_t`` and
+ *   derivatives.
+ * - ``d[234]`` - For a dentry name (optionally 2-4 last components).
+ * - ``D[234]`` - Same as 'd' but for a struct file.
+ * - ``g`` - For ``block_device`` name (gendisk + partition number).
+ * - ``C[n]`` - For a clock, it prints the name (Common Clock Framework) or
+ *   address (legacy clock framework) of the clock. ``[n]`` is optional.
+ * - ``Cr`` - For a clock, it prints the current rate of the clock.
+ * - ``G`` - For flags to be printed as a collection of symbolic strings that
+ *   would construct the specific value.
+ * - ``O`` - For a kobject based struct (device node).
+ * - ``x`` - For printing the address. Equivalent to ``%lx``.
+ */
+
 /*
  * Show a '%p' thing.  A kernel extension is that the '%p' is followed
  * by an extra set of alphanumeric characters that are extended format
  * specifiers.
  *
+ * Please see Documentation/printk-formats.rst for fuller description
+ * of specifier extensions. Also please update this file when making
+ * changes.
+ *
  * Please update scripts/checkpatch.pl when adding/removing conversion
  * characters.  (Search for "check for vsprintf extension").
  *
- * Right now we handle:
- *
- * - 'F' For symbolic function descriptor pointers with offset
- * - 'f' For simple symbolic function names without offset
- * - 'S' For symbolic direct pointers with offset
- * - 's' For symbolic direct pointers without offset
- * - '[FfSs]R' as above with __builtin_extract_return_addr() translation
- * - 'B' For backtraced symbolic direct pointers with offset
- * - 'R' For decoded struct resource, e.g., [mem 0x0-0x1f 64bit pref]
- * - 'r' For raw struct resource, e.g., [mem 0x0-0x1f flags 0x201]
- * - 'b[l]' For a bitmap, the number of bits is determined by the field
- *       width which must be explicitly specified either as part of the
- *       format string '%32b[l]' or through '%*b[l]', [l] selects
- *       range-list format instead of hex format
- * - 'M' For a 6-byte MAC address, it prints the address in the
- *       usual colon-separated hex notation
- * - 'm' For a 6-byte MAC address, it prints the hex address without colons
- * - 'MF' For a 6-byte MAC FDDI address, it prints the address
- *       with a dash-separated hex notation
- * - '[mM]R' For a 6-byte MAC address, Reverse order (Bluetooth)
- * - 'I' [46] for IPv4/IPv6 addresses printed in the usual way
- *       IPv4 uses dot-separated decimal without leading 0's (1.2.3.4)
- *       IPv6 uses colon separated network-order 16 bit hex with leading 0's
- *       [S][pfs]
- *       Generic IPv4/IPv6 address (struct sockaddr *) that falls back to
- *       [4] or [6] and is able to print port [p], flowinfo [f], scope [s]
- * - 'i' [46] for 'raw' IPv4/IPv6 addresses
- *       IPv6 omits the colons (01020304...0f)
- *       IPv4 uses dot-separated decimal with leading 0's (010.123.045.006)
- *       [S][pfs]
- *       Generic IPv4/IPv6 address (struct sockaddr *) that falls back to
- *       [4] or [6] and is able to print port [p], flowinfo [f], scope [s]
- * - '[Ii][4S][hnbl]' IPv4 addresses in host, network, big or little endian order
- * - 'I[6S]c' for IPv6 addresses printed as specified by
- *       http://tools.ietf.org/html/rfc5952
- * - 'E[achnops]' For an escaped buffer, where rules are defined by combination
- *                of the following flags (see string_escape_mem() for the
- *                details):
- *                  a - ESCAPE_ANY
- *                  c - ESCAPE_SPECIAL
- *                  h - ESCAPE_HEX
- *                  n - ESCAPE_NULL
- *                  o - ESCAPE_OCTAL
- *                  p - ESCAPE_NP
- *                  s - ESCAPE_SPACE
- *                By default ESCAPE_ANY_NP is used.
- * - 'U' For a 16 byte UUID/GUID, it prints the UUID/GUID in the form
- *       "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"
- *       Options for %pU are:
- *         b big endian lower case hex (default)
- *         B big endian UPPER case hex
- *         l little endian lower case hex
- *         L little endian UPPER case hex
- *           big endian output byte order is:
- *             [0][1][2][3]-[4][5]-[6][7]-[8][9]-[10][11][12][13][14][15]
- *           little endian output byte order is:
- *             [3][2][1][0]-[5][4]-[7][6]-[8][9]-[10][11][12][13][14][15]
- * - 'V' For a struct va_format which contains a format string * and va_list *,
- *       call vsnprintf(->format, *->va_list).
- *       Implements a "recursive vsnprintf".
- *       Do not use this feature without some mechanism to verify the
- *       correctness of the format string and va_list arguments.
- * - 'K' For a kernel pointer that should be hidden from unprivileged users
- * - 'NF' For a netdev_features_t
- * - 'h[CDN]' For a variable-length buffer, it prints it as a hex string with
- *            a certain separator (' ' by default):
- *              C colon
- *              D dash
- *              N no separator
- *            The maximum supported length is 64 bytes of the input. Consider
- *            to use print_hex_dump() for the larger input.
- * - 'a[pd]' For address types [p] phys_addr_t, [d] dma_addr_t and derivatives
- *           (default assumed to be phys_addr_t, passed by reference)
- * - 'd[234]' For a dentry name (optionally 2-4 last components)
- * - 'D[234]' Same as 'd' but for a struct file
- * - 'g' For block_device name (gendisk + partition number)
- * - 'C' For a clock, it prints the name (Common Clock Framework) or address
- *       (legacy clock framework) of the clock
- * - 'Cn' For a clock, it prints the name (Common Clock Framework) or address
- *        (legacy clock framework) of the clock
- * - 'Cr' For a clock, it prints the current rate of the clock
- * - 'G' For flags to be printed as a collection of symbolic strings that would
- *       construct the specific value. Supported flags given by option:
- *       p page flags (see struct page) given as pointer to unsigned long
- *       g gfp flags (GFP_* and __GFP_*) given as pointer to gfp_t
- *       v vma flags (VM_*) given as pointer to unsigned long
- * - 'O' For a kobject based struct. Must be one of the following:
- *       - 'OF[fnpPcCF]'  For a device tree object
- *                        Without any optional arguments prints the full_name
- *                        f device node full_name
- *                        n device node name
- *                        p device node phandle
- *                        P device node path spec (name + @unit)
- *                        F device node flags
- *                        c major compatible string
- *                        C full compatible string
- *
- * - 'x' For printing the address. Equivalent to "%lx".
- *
- * ** Please update also Documentation/printk-formats.txt when making changes **
- *
  * Note: The difference between 'S' and 'F' is that on ia64 and ppc64
  * function pointers are really function descriptors, which contain a
  * pointer to the real address.
-- 
2.7.4

Re: [PATCH] doc: convert printk-formats.txt to rst

[Markus Heiser]

> Am 06.12.2017 um 02:45 schrieb Tobin C. Harding <me@tobin.cc>:
> 
> Documentation/printk-formats.txt is a candidate for conversion to
> ReStructuredText format. Some effort has already been made to do this
> conversion even thought the suffix is currently .txt
> 
> Changes required to complete conversion
> 
> - Add double backticks where needed.
> - Add entry to Documentation/index.rst
> - Use flat-table instead of ASCII table.

[...]

> +=============================================
> +How to Get ``printk`` Format Specifiers Right
> +=============================================
> 
> :Author: Randy Dunlap <rdunlap@infradead.org>
> :Author: Andrew Murray <amurray@mpc-data.co.uk>
> @@ -8,56 +8,91 @@ How to get printk format specifiers right
> Integer types
> =============
> 
> -::
> +For printing integer types, we have the following format specifiers:
> +		
> +   .. flat-table:: 
> +      :widths: 2 2
> +
> +      * - **Type**
> +	- **Specifier**
> +
> +      * - ``int``
> +        - ``%d`` or ``%x``
> +
> +      * - ``unsigned int``
> +	- ``%u`` or ``%x``
> +
> +      * - ``long``
> +	- ``%ld`` or ``%lx``
> +
> +      * - ``unsigned long``
> +	- ``%lu`` or ``%lx``
> +
> +      * - ``long long``
> +	- ``%lld`` or ``%llx``
> 
> -	If variable is of Type,		use printk format specifier:
> -	------------------------------------------------------------
> -		int			%d or %x
> -		unsigned int		%u or %x
> -		long			%ld or %lx
> -		unsigned long		%lu or %lx
> -		long long		%lld or %llx
> -		unsigned long long	%llu or %llx
> -		size_t			%zu or %zx
> -		ssize_t			%zd or %zx
> -		s32			%d or %x
> -		u32			%u or %x
> -		s64			%lld or %llx
> -		u64			%llu or %llx
> -
> -If <type> is dependent on a config option for its size (e.g., ``sector_t``,
> +      * - ``unsigned long long``
> +	- ``%llu`` or ``%llx``
> +
> +      * - ``size_t``
> +	- ``%zu`` or ``%zx``
> +
> +      * - ``ssize_t``
> +	- ``%zd`` or ``%zx``
> +
> +      * - ``s32``
> +	- ``%d`` or ``%x``
> +
> +      * - ``u32``
> +	- ``%u`` or ``%x``
> +
> +      * - ``s64``
> +	- ``%lld`` or ``%llx``
> +
> +      * - ``u64``
> +	- ``%llu`` or ``%llx``
> +
> +

Thanks!

just a question .. might it be better we stay with ASCII table
in cases like this. I guess this table won't changed often.
The flat-table directive is good for big and therefore frequently 
changed tables where a small precise diff reduce the patch.
But flat-table is also hard to read in plain text. Its a balancing
and thats my opinion, lets hear what other say ...

-- Markus --

Re: [PATCH] doc: convert printk-formats.txt to rst

[Joe Perches]

On Wed, 2017-12-06 at 08:11 +0100, Markus Heiser wrote:
> > Am 06.12.2017 um 02:45 schrieb Tobin C. Harding <me@tobin.cc>:
> > Documentation/printk-formats.txt is a candidate for conversion to
> > ReStructuredText format. Some effort has already been made to do this
> > conversion even thought the suffix is currently .txt
[]
> just a question .. might it be better we stay with ASCII table
> in cases like this. I guess this table won't changed often.
> The flat-table directive is good for big and therefore frequently 
> changed tables where a small precise diff reduce the patch.
> But flat-table is also hard to read in plain text. Its a balancing
> and thats my opinion, lets hear what other say ...

I think the proposed conversion is unreadable
and the original table quite clear.

Re: [PATCH] doc: convert printk-formats.txt to rst

[Randy Dunlap]

On 12/05/2017 11:35 PM, Joe Perches wrote:
> On Wed, 2017-12-06 at 08:11 +0100, Markus Heiser wrote:
>>> Am 06.12.2017 um 02:45 schrieb Tobin C. Harding <me@tobin.cc>:
>>> Documentation/printk-formats.txt is a candidate for conversion to
>>> ReStructuredText format. Some effort has already been made to do this
>>> conversion even thought the suffix is currently .txt
> []
>> just a question .. might it be better we stay with ASCII table
>> in cases like this. I guess this table won't changed often.
>> The flat-table directive is good for big and therefore frequently 
>> changed tables where a small precise diff reduce the patch.
>> But flat-table is also hard to read in plain text. Its a balancing
>> and thats my opinion, lets hear what other say ...
> 
> I think the proposed conversion is unreadable
> and the original table quite clear.

Agree.


-- 
~Randy

Re: [PATCH] doc: convert printk-formats.txt to rst

[Randy Dunlap]

On 12/05/2017 05:45 PM, Tobin C. Harding wrote:
> Documentation/printk-formats.txt is a candidate for conversion to
> ReStructuredText format. Some effort has already been made to do this
> conversion even thought the suffix is currently .txt
> 
> Changes required to complete conversion
> 
> - Add double backticks where needed.
> - Add entry to Documentation/index.rst
> - Use flat-table instead of ASCII table.
> - Fix minor grammatical errors.
> - Capitalize headers and correctly order heading adornments.

That's a style choice and an unneeded change (referring to Capitalize headers).

> - Use 'Passed by reference' uniformly.
> - Update pointer documentation around %px specifier.
> - Fix erroneous double backticks (to commas).
> - Simplify documentation for kobject.
> - Convert lib/vsnprintf.c function docs to use kernel-docs and
>   include in Documentation/printk-formats.rst

good idea.

> 
> Signed-off-by: Tobin C. Harding <me@tobin.cc>
> ---
> 
> The last two need special reviewing please. In particular the conversion
> of kernel-docs in vsnprintf.c attempt was made to reduce documentation
> duplication with comments in the source code being simplified in order
> to be suitable for inclusion in Documentation/printk-formats.rst
> 
> I used -M when formatting the patch. If you don't like the diff with
> this flag just holla.
> 
> thanks,
> Tobin.
> 
>  Documentation/index.rst                            |  10 +
>  .../{printk-formats.txt => printk-formats.rst}     | 295 ++++++++++++---------
>  lib/vsprintf.c                                     | 160 +++++------
>  3 files changed, 235 insertions(+), 230 deletions(-)
>  rename Documentation/{printk-formats.txt => printk-formats.rst} (61%)

> diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.rst
> similarity index 61%
> rename from Documentation/printk-formats.txt
> rename to Documentation/printk-formats.rst
> index aa0a776c817a..51449d213748 100644
> --- a/Documentation/printk-formats.txt
> +++ b/Documentation/printk-formats.rst
> @@ -1,6 +1,6 @@
> -=========================================
> -How to get printk format specifiers right
> -=========================================
> +=============================================
> +How to Get ``printk`` Format Specifiers Right
> +=============================================
>  
>  :Author: Randy Dunlap <rdunlap@infradead.org>
>  :Author: Andrew Murray <amurray@mpc-data.co.uk>
> @@ -8,56 +8,91 @@ How to get printk format specifiers right
>  Integer types
>  =============
>  
> -::
> +For printing integer types, we have the following format specifiers:
> +		
> +   .. flat-table:: 
> +      :widths: 2 2
> +
> +      * - **Type**
> +	- **Specifier**
> +
> +      * - ``int``
> +        - ``%d`` or ``%x``
> +
> +      * - ``unsigned int``
> +	- ``%u`` or ``%x``
> +
> +      * - ``long``
> +	- ``%ld`` or ``%lx``
> +
> +      * - ``unsigned long``
> +	- ``%lu`` or ``%lx``
> +
> +      * - ``long long``
> +	- ``%lld`` or ``%llx``
>  
> -	If variable is of Type,		use printk format specifier:
> -	------------------------------------------------------------
> -		int			%d or %x
> -		unsigned int		%u or %x
> -		long			%ld or %lx
> -		unsigned long		%lu or %lx
> -		long long		%lld or %llx
> -		unsigned long long	%llu or %llx
> -		size_t			%zu or %zx
> -		ssize_t			%zd or %zx
> -		s32			%d or %x
> -		u32			%u or %x
> -		s64			%lld or %llx
> -		u64			%llu or %llx
> -
> -If <type> is dependent on a config option for its size (e.g., ``sector_t``,
> +      * - ``unsigned long long``
> +	- ``%llu`` or ``%llx``
> +
> +      * - ``size_t``
> +	- ``%zu`` or ``%zx``
> +
> +      * - ``ssize_t``
> +	- ``%zd`` or ``%zx``
> +
> +      * - ``s32``
> +	- ``%d`` or ``%x``
> +
> +      * - ``u32``
> +	- ``%u`` or ``%x``
> +
> +      * - ``s64``
> +	- ``%lld`` or ``%llx``
> +
> +      * - ``u64``
> +	- ``%llu`` or ``%llx``
> +
> +
> +If ``<type>`` is dependent on a config option for its size (e.g., ``sector_t``,
>  ``blkcnt_t``) or is architecture-dependent for its size (e.g., ``tcflag_t``),
>  use a format specifier of its largest possible type and explicitly cast to it.
>  
>  Example::
>  
> -	printk("test: sector number/total blocks: %llu/%llu\n",
> -		(unsigned long long)sector, (unsigned long long)blockcount);
> +	printk("test: total blocks: %llu\n", (unsigned long long)blockcount);
>  
> -Reminder: ``sizeof()`` result is of type ``size_t``.
> +Reminder: ``sizeof()`` returns type ``size_t``.
>  
> -The kernel's printf does not support ``%n``. For obvious reasons, floating
> +The kernel's printf does not support ``%n``. For obvious reasons floating
>  point formats (``%e, %f, %g, %a``) are also not recognized. Use of any
>  unsupported specifier or length qualifier results in a WARN and early
> -return from vsnprintf.
> -
> -Raw pointer value SHOULD be printed with %p. The kernel supports
> -the following extended format specifiers for pointer types:
> +return from ``vsnprintf()``.
>  
>  Pointer Types
>  =============
>  
> -Pointers printed without a specifier extension (i.e unadorned %p) are
> -hashed to give a unique identifier without leaking kernel addresses to user
> -space. On 64 bit machines the first 32 bits are zeroed. If you _really_
> -want the address see %px below.
> +A raw pointer value may be printed with ``%p`` which will hash the address
> +before printing. The Kernel also supports extended specifiers for printing
> +pointers of different types.
> +
> +.. kernel-doc:: lib/vsprintf.c
> +     :doc: Extended Format Pointer Specifiers
> +
> +
> +Plain Pointers
> +--------------
>  
>  ::
>  
>  	%p	abcdef12 or 00000000abcdef12
>  
> +Pointers printed without a specifier extension (i.e unadorned ``%p``) are
> +hashed to give a unique identifier without leaking kernel addresses to user
> +space. On 64 bit machines the first 32 bits are zeroed. If you *really*

             64-bit

> +want the address see ``%px`` below.
> +
>  Symbols/Function Pointers
> -=========================
> +-------------------------
>  
>  ::
>  
> @@ -69,61 +104,60 @@ Symbols/Function Pointers
>  	%ps	versatile_init
>  	%pB	prev_fn_of_versatile_init+0x88/0x88
>  
> -The ``F`` and ``f`` specifiers are for printing function pointers,
> -for example, f->func, &gettimeofday. They have the same result as
> -``S`` and ``s`` specifiers. But they do an extra conversion on
> -ia64, ppc64 and parisc64 architectures where the function pointers
> -are actually function descriptors.
> +The ``F`` and ``f`` specifiers are for printing function pointers, for
> +example, ``f->func``, ``&gettimeofday``. They have the same result as ``S``
> +and ``s`` specifiers. But they do an extra conversion on ia64, ppc64 and
> +parisc64 architectures where the function pointers are actually function
> +descriptors.
>  
>  The ``S`` and ``s`` specifiers can be used for printing symbols
> -from direct addresses, for example, __builtin_return_address(0),
> -(void *)regs->ip. They result in the symbol name with (``S``) or
> +from direct addresses, for example, ``__builtin_return_address(0)``,
> +``(void *)regs->ip``. They result in the symbol name with (``S``) or
>  without (``s``) offsets. If KALLSYMS are disabled then the symbol
>  address is printed instead.
>  
>  The ``B`` specifier results in the symbol name with offsets and should be
>  used when printing stack backtraces. The specifier takes into
>  consideration the effect of compiler optimisations which may occur
> -when tail-call``s are used and marked with the noreturn GCC attribute.
> +when tail-call's are used and marked with the ``noreturn`` GCC attribute.
>  
>  Examples::
>  
>  	printk("Going to call: %pF\n", gettimeofday);
>  	printk("Going to call: %pF\n", p->func);
>  	printk("%s: called from %pS\n", __func__, (void *)_RET_IP_);
> -	printk("%s: called from %pS\n", __func__,
> -				(void *)__builtin_return_address(0));
> +	printk("%s: called from %pS\n", __func__, (void *)__builtin_return_address(0));
>  	printk("Faulted at %pS\n", (void *)regs->ip);
>  	printk(" %s%pB\n", (reliable ? "" : "? "), (void *)*stack);
>  
>  Kernel Pointers
> -===============
> +---------------
>  
>  ::
>  
>  	%pK	01234567 or 0123456789abcdef
>  
>  For printing kernel pointers which should be hidden from unprivileged
> -users. The behaviour of ``%pK`` depends on the ``kptr_restrict sysctl`` - see
> -Documentation/sysctl/kernel.txt for more details.
> +users. The behaviour of ``%pK`` depends on the ``kptr_restrict`` sysctl -
> +see ``Documentation/sysctl/kernel.txt`` for more details.
>  
>  Unmodified Addresses
> -====================
> +--------------------
>  
>  ::
>  
>  	%px	01234567 or 0123456789abcdef
>  
> -For printing pointers when you _really_ want to print the address. Please
> +For printing pointers when you *really* want to print the address. Please
>  consider whether or not you are leaking sensitive information about the
> -Kernel layout in memory before printing pointers with %px. %px is
> -functionally equivalent to %lx. %px is preferred to %lx because it is more
> -uniquely grep'able. If, in the future, we need to modify the way the Kernel
> -handles printing pointers it will be nice to be able to find the call
> -sites.
> +kernel memory layout before printing pointers with ``%px``. ``%px`` is
> +functionally equivalent to ``%lx`` (or ``%lu``). ``%px``, however, is
> +preferable because it is more uniquely grep'able. If, in the future, we need
> +to modify the way the Kernel handles printing pointers we will be better
> +equipped to find the call sites.
>  
>  Struct Resources
> -================
> +----------------
>  
>  ::
>  
> @@ -132,12 +166,13 @@ Struct Resources
>  	%pR	[mem 0x60000000-0x6fffffff pref] or
>  		[mem 0x0000000060000000-0x000000006fffffff pref]
>  
> -For printing struct resources. The ``R`` and ``r`` specifiers result in a
> +For printing ``struct resources``. The ``R`` and ``r`` specifiers result in a
>  printed resource with (``R``) or without (``r``) a decoded flags member.
> +
>  Passed by reference.
>  
> -Physical addresses types ``phys_addr_t``
> -========================================
> +Physical Address Types ``phys_addr_t``
> +--------------------------------------
>  
>  ::
>  
> @@ -145,20 +180,24 @@ Physical addresses types ``phys_addr_t``
>  
>  For printing a ``phys_addr_t`` type (and its derivatives, such as
>  ``resource_size_t``) which can vary based on build options, regardless of
> -the width of the CPU data path. Passed by reference.
> +the width of the CPU data path.
> +
> +Passed by reference.
>  
> -DMA addresses types ``dma_addr_t``
> -==================================
> +DMA Address Types ``dma_addr_t``
> +--------------------------------
>  
>  ::
>  
>  	%pad	0x01234567 or 0x0123456789abcdef
>  
>  For printing a ``dma_addr_t`` type which can vary based on build options,
> -regardless of the width of the CPU data path. Passed by reference.
> +regardless of the width of the CPU data path.
>  
> -Raw buffer as an escaped string
> -===============================
> +Passed by reference.
> +
> +Raw Buffer as an Escaped String
> +-------------------------------
>  
>  ::
>  
> @@ -168,7 +207,7 @@ For printing raw buffer as an escaped string. For the following buffer::
>  
>  		1b 62 20 5c 43 07 22 90 0d 5d
>  
> -few examples show how the conversion would be done (the result string
> +A few examples show how the conversion would be done (the result string
>  without surrounding quotes)::
>  
>  		%*pE		"\eb \C\a"\220\r]"
> @@ -194,8 +233,8 @@ printing SSIDs.
>  
>  If field width is omitted the 1 byte only will be escaped.

                             then
I think...

>  
> -Raw buffer as a hex string
> -==========================
> +Raw Buffer as a Hex String
> +--------------------------
>  
>  ::
>  
> @@ -205,11 +244,11 @@ Raw buffer as a hex string
>  	%*phN	000102 ... 3f
>  
>  For printing a small buffers (up to 64 bytes long) as a hex string with
> -certain separator. For the larger buffers consider to use
> +certain separator. For the larger buffers consider using
>  :c:func:`print_hex_dump`.
>  
> -MAC/FDDI addresses
> -==================
> +MAC/FDDI Addresses
> +------------------
>  
>  ::
>  
> @@ -233,8 +272,8 @@ of Bluetooth addresses which are in the little endian order.
>  
>  Passed by reference.
>  
> -IPv4 addresses
> -==============
> +IPv4 Addresses
> +--------------
>  
>  ::
>  
> @@ -252,8 +291,8 @@ no specifier is provided the default network/big endian order is used.
>  
>  Passed by reference.
>  
> -IPv6 addresses
> -==============
> +IPv6 Addresses
> +--------------
>  
>  ::
>  
> @@ -271,8 +310,8 @@ http://tools.ietf.org/html/rfc5952
>  
>  Passed by reference.
>  
> -IPv4/IPv6 addresses (generic, with port, flowinfo, scope)
> -=========================================================
> +IPv4/IPv6 Addresses (generic, with port, flowinfo or scope)
> +---------------------------------------------------------------

I prefer the additional (Oxford) comma.
and why is the --- line longer than the header?

>  
>  ::
>  
> @@ -282,8 +321,8 @@ IPv4/IPv6 addresses (generic, with port, flowinfo, scope)
>  	%pISpc	1.2.3.4:12345	or [1:2:3:4:5:6:7:8]:12345
>  	%p[Ii]S[pfschnbl]
>  
> -For printing an IP address without the need to distinguish whether it``s
> -of type AF_INET or AF_INET6, a pointer to a valid ``struct sockaddr``,
> +For printing an IP address without the need to distinguish whether it's
> +of type AF_INET or AF_INET6. A pointer to a valid ``struct sockaddr``,
>  specified through ``IS`` or ``iS``, can be passed to this format specifier.
>  
>  The additional ``p``, ``f``, and ``s`` specifiers are used to specify port
> @@ -308,8 +347,8 @@ Further examples::
>  	%pISsc		1.2.3.4		or [1:2:3:4:5:6:7:8]%1234567890
>  	%pISpfc		1.2.3.4:12345	or [1:2:3:4:5:6:7:8]:12345/123456789
>  
> -UUID/GUID addresses
> -===================
> +UUID/GUID Addresses
> +-------------------
>  
>  ::
>  
> @@ -318,18 +357,18 @@ UUID/GUID addresses
>  	%pUl	03020100-0504-0706-0809-0a0b0c0e0e0f
>  	%pUL	03020100-0504-0706-0809-0A0B0C0E0E0F
>  
> -For printing 16-byte UUID/GUIDs addresses. The additional 'l', 'L',
> -'b' and 'B' specifiers are used to specify a little endian order in
> -lower ('l') or upper case ('L') hex characters - and big endian order
> -in lower ('b') or upper case ('B') hex characters.
> +For printing 16-byte UUID/GUIDs addresses. The additional ``l``, ``L``,
> +``b`` and ``B`` specifiers are used to specify a little endian order in
> +lower (``l``) or upper case (``L``) hex digits - and big endian order
> +in lower (``b``) or upper case (``B``) hex digits.
>  
>  Where no additional specifiers are used the default big endian
> -order with lower case hex characters will be printed.
> +order with lower case hex digits will be printed.

digits could imply base 10. but no big deal.

>  
>  Passed by reference.
>  
> -dentry names
> -============
> +Dentry Names
> +------------
>  
>  ::
>  
> @@ -343,24 +382,24 @@ equivalent of ``%s`` ``dentry->d_name.name`` we used to use, ``%pd<n>`` prints
>  
>  Passed by reference.
>  
> -block_device names
> -==================
> +block_device Names
> +------------------
>  
>  ::
>  
>  	%pg	sda, sda1 or loop0p1
>  
> -For printing name of block_device pointers.
> +For printing name of ``block_device`` pointers.
>  
>  struct va_format
> -================
> +----------------
>  
>  ::
>  
>  	%pV
>  
> -For printing struct va_format structures. These contain a format string
> -and va_list as follows::
> +For printing ``struct va_format`` structures. These contain a format string
> +and ``va_list`` as follows::
>  
>  	struct va_format {
>  		const char *fmt;
> @@ -370,36 +409,33 @@ and va_list as follows::
>  Implements a "recursive vsnprintf".
>  
>  Do not use this feature without some mechanism to verify the
> -correctness of the format string and va_list arguments.
> +correctness of the format string and ``va_list`` arguments.
>  
>  Passed by reference.
>  
>  kobjects
> -========
> -
> +--------
> +	
>  ::
>  
> -	%pO
> +	%pOF[fnpPcCF]
>  
> -	Base specifier for kobject based structs. Must be followed with
> -	character for specific type of kobject as listed below:
>  
> -	Device tree nodes:
> +For printing kobject based structs (device nodes). Default behaviour is
> +equivalent to ``%pOFf``.
>  
> -	%pOF[fnpPcCF]
> +	- ``f`` device node full_name
> +	- ``n`` device node name
> +	- ``p`` device node phandle
> +	- ``P`` device node path spec (name + @unit)
> +	- ``F`` device node flags
> +	- ``c`` major compatible string
> +	- ``C`` full compatible string
>  
> -	For printing device tree nodes. The optional arguments are:
> -	    f device node full_name
> -	    n device node name
> -	    p device node phandle
> -	    P device node path spec (name + @unit)
> -	    F device node flags
> -	    c major compatible string
> -	    C full compatible string
> -	Without any arguments prints full_name (same as %pOFf)
> -	The separator when using multiple arguments is ':'
> +The separator when using multiple arguments is ``:``
>  
> -	Examples:
> +Examples:
> +::
>  
>  	%pOF	/foo/bar@0			- Node full name
>  	%pOFf	/foo/bar@0			- Same as above
> @@ -412,11 +448,10 @@ kobjects
>  							P - Populated
>  							B - Populated bus
>  
> -	Passed by reference.
> -
> +Passed by reference.
>  
>  struct clk
> -==========
> +----------
>  
>  ::
>  
> @@ -424,14 +459,14 @@ struct clk
>  	%pCn	pll1
>  	%pCr	1560000000
>  
> -For printing struct clk structures. ``%pC`` and ``%pCn`` print the name
> +For printing ``struct clk structures``. ``%pC`` and ``%pCn`` print the name
>  (Common Clock Framework) or address (legacy clock framework) of the
>  structure; ``%pCr`` prints the current clock rate.
>  
>  Passed by reference.
>  
> -bitmap and its derivatives such as cpumask and nodemask
> -=======================================================
> +Bitmap and its Derivatives (such as cpumask and nodemask)
> +---------------------------------------------------------
>  
>  ::
>  
> @@ -439,13 +474,13 @@ bitmap and its derivatives such as cpumask and nodemask
>  	%*pbl	0,3-6,8-10
>  
>  For printing bitmap and its derivatives such as cpumask and nodemask,
> -``%*pb`` output the bitmap with field width as the number of bits and ``%*pbl``
> -output the bitmap as range list with field width as the number of bits.
> +``%*pb`` outputs the bitmap with field width as the number of bits and ``%*pbl``
> +outputs the bitmap as range list with field width as the number of bits.
>  
>  Passed by reference.
>  
> -Flags bitfields such as page flags, gfp_flags
> -=============================================
> +Flags Bitfields (such as page flags, gfp_flags)
> +-----------------------------------------------
>  
>  ::
>  
> @@ -459,25 +494,27 @@ character. Currently supported are [p]age flags, [v]ma_flags (both
>  expect ``unsigned long *``) and [g]fp_flags (expects ``gfp_t *``). The flag
>  names and print order depends on the particular	type.
>  
> -Note that this format should not be used directly in :c:func:`TP_printk()` part
> -of a tracepoint. Instead, use the ``show_*_flags()`` functions from
> -<trace/events/mmflags.h>.
> +Note that this format should not be used directly in the
> +:c:func:`TP_printk()` part of a tracepoint. Instead, use the
> +``show_*_flags()`` functions from ``<trace/events/mmflags.h>``. 
>  
>  Passed by reference.
>  
> -Network device features
> -=======================
> +Network Device Features
> +-----------------------
>  
>  ::
>  
>  	%pNF	0x000000000000c000
>  
> -For printing netdev_features_t.
> +For printing ``netdev_features_t``.
>  
>  Passed by reference.
>  
> -If you add other ``%p`` extensions, please extend lib/test_printf.c with
> -one or more test cases, if at all feasible.
> +Thanks
> +======
>  
> +If you add other ``%p`` extensions, please extend ``lib/test_printf.c``
> +with one or more test cases, if at all feasible.
>  
>  Thank you for your cooperation and attention.
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 01c3957b2de6..f9247b78e8ef 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1727,115 +1727,73 @@ static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec)
>  	return number(buf, end, hashval, spec);
>  }
>  
> +/**
> + * DOC: Extended Format Pointer Specifiers
> + *
> + * Briefly we handle the following extensions:
> + *
> + * - ``F`` - For symbolic function descriptor pointers with offset.
> + * - ``f`` - For simple symbolic function names without offset.
> + *
> + * - ``S`` - For symbolic direct pointers with offset.
> + * - ``s`` - For symbolic direct pointers without offset.
> + * - ``[FfSs]R`` - As above with ``__builtin_extract_return_addr()`` translation.
> + * - ``B`` - For backtraced symbolic direct pointers with offset.
> + * - ``R`` - For decoded struct resource, e.g., [mem 0x0-0x1f 64bit pref].
> + * - ``r`` - For raw struct resource, e.g., [mem 0x0-0x1f flags 0x201].
> + * - ``b[l]`` - For a bitmap, the number of bits is determined by the field
> + *   width which must be explicitly specified either as part of the format
> + *   string ``32b[l]`` or through ``*b[l]``, ``[l]`` selects range-list format
> + *   instead of hex format.
> + * - ``M`` - For a 6-byte MAC address, it prints the address in the usual
> + *   colon-separated hex notation.
> + * - ``m`` - For a 6-byte MAC address, it prints the hex address without colons.
> + * - ``MF`` - For a 6-byte MAC FDDI address, it prints the address with a
> + *   dash-separated hex notation.
> + * - ``[mM]R`` - For a 6-byte MAC address, Reverse order (Bluetooth).
> + * - ``I[46]`` - For IPv4/IPv6 addresses printed in the usual way.
> + * - ``I[S][pfs]`` - For generic IPv4/IPv6 address (struct sockaddr *) that falls
> + *   back to ``[4]`` or ``[6]`` and is able to print port ``[p]``,
> + *   flowinfo ``[f]``, scope ``[s]``.
> + * - ``i[46]`` - For 'raw' IPv4/IPv6 addresses IPv6 omits the colons (01020304...0f)
> + *   IPv4 uses dot-separated decimal with leading 0's (010.123.045.006).
> + * - ``i[S][pfs]`` - For generic IPv4/IPv6 address (struct sockaddr *) that falls back
> + *   to ``[4]`` or ``[6]`` (``[pfs]`` as above).
> + * - ``[Ii][4S][hnbl]`` - For IPv4 addresses in host, network, big or little endian order.
> + * - ``I[6S]c`` - For IPv6 addresses printed as per http://tools.ietf.org/html/rfc5952.
> + * - ``E[achnops]`` - For an escaped buffer.
> + * - ``U`` - For a 16 byte UUID/GUID.
> + * - ``V`` - For a ``struct va_format`` which contains a format ``string *``
> + *   and ``va_list *``.
> + * - ``K`` -  For a kernel pointer that should be hidden from unprivileged users.
> + * - ``NF`` - For a ``netdev_features_t``.
> + * - ``h[CDN]`` - For a variable-length buffer.
> + * - ``a[pd]`` - For address types ``[p] phys_addr_t``, ``[d] dma_addr_t`` and
> + *   derivatives.
> + * - ``d[234]`` - For a dentry name (optionally 2-4 last components).
> + * - ``D[234]`` - Same as 'd' but for a struct file.
> + * - ``g`` - For ``block_device`` name (gendisk + partition number).
> + * - ``C[n]`` - For a clock, it prints the name (Common Clock Framework) or
> + *   address (legacy clock framework) of the clock. ``[n]`` is optional.
> + * - ``Cr`` - For a clock, it prints the current rate of the clock.
> + * - ``G`` - For flags to be printed as a collection of symbolic strings that
> + *   would construct the specific value.
> + * - ``O`` - For a kobject based struct (device node).
> + * - ``x`` - For printing the address. Equivalent to ``%lx``.
> + */
> +
>  /*
>   * Show a '%p' thing.  A kernel extension is that the '%p' is followed
>   * by an extra set of alphanumeric characters that are extended format
>   * specifiers.
>   *
> + * Please see Documentation/printk-formats.rst for fuller description
> + * of specifier extensions. Also please update this file when making

"this file" is the file that I am reading?  or could it be "that file"?

> + * changes.
> + *
>   * Please update scripts/checkpatch.pl when adding/removing conversion
>   * characters.  (Search for "check for vsprintf extension").
>   *
> - * Right now we handle:
> - *
> - * - 'F' For symbolic function descriptor pointers with offset
> - * - 'f' For simple symbolic function names without offset
> - * - 'S' For symbolic direct pointers with offset
> - * - 's' For symbolic direct pointers without offset
> - * - '[FfSs]R' as above with __builtin_extract_return_addr() translation
> - * - 'B' For backtraced symbolic direct pointers with offset
> - * - 'R' For decoded struct resource, e.g., [mem 0x0-0x1f 64bit pref]
> - * - 'r' For raw struct resource, e.g., [mem 0x0-0x1f flags 0x201]
> - * - 'b[l]' For a bitmap, the number of bits is determined by the field
> - *       width which must be explicitly specified either as part of the
> - *       format string '%32b[l]' or through '%*b[l]', [l] selects
> - *       range-list format instead of hex format
> - * - 'M' For a 6-byte MAC address, it prints the address in the
> - *       usual colon-separated hex notation
> - * - 'm' For a 6-byte MAC address, it prints the hex address without colons
> - * - 'MF' For a 6-byte MAC FDDI address, it prints the address
> - *       with a dash-separated hex notation
> - * - '[mM]R' For a 6-byte MAC address, Reverse order (Bluetooth)
> - * - 'I' [46] for IPv4/IPv6 addresses printed in the usual way
> - *       IPv4 uses dot-separated decimal without leading 0's (1.2.3.4)
> - *       IPv6 uses colon separated network-order 16 bit hex with leading 0's
> - *       [S][pfs]
> - *       Generic IPv4/IPv6 address (struct sockaddr *) that falls back to
> - *       [4] or [6] and is able to print port [p], flowinfo [f], scope [s]
> - * - 'i' [46] for 'raw' IPv4/IPv6 addresses
> - *       IPv6 omits the colons (01020304...0f)
> - *       IPv4 uses dot-separated decimal with leading 0's (010.123.045.006)
> - *       [S][pfs]
> - *       Generic IPv4/IPv6 address (struct sockaddr *) that falls back to
> - *       [4] or [6] and is able to print port [p], flowinfo [f], scope [s]
> - * - '[Ii][4S][hnbl]' IPv4 addresses in host, network, big or little endian order
> - * - 'I[6S]c' for IPv6 addresses printed as specified by
> - *       http://tools.ietf.org/html/rfc5952
> - * - 'E[achnops]' For an escaped buffer, where rules are defined by combination
> - *                of the following flags (see string_escape_mem() for the
> - *                details):
> - *                  a - ESCAPE_ANY
> - *                  c - ESCAPE_SPECIAL
> - *                  h - ESCAPE_HEX
> - *                  n - ESCAPE_NULL
> - *                  o - ESCAPE_OCTAL
> - *                  p - ESCAPE_NP
> - *                  s - ESCAPE_SPACE
> - *                By default ESCAPE_ANY_NP is used.
> - * - 'U' For a 16 byte UUID/GUID, it prints the UUID/GUID in the form
> - *       "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"
> - *       Options for %pU are:
> - *         b big endian lower case hex (default)
> - *         B big endian UPPER case hex
> - *         l little endian lower case hex
> - *         L little endian UPPER case hex
> - *           big endian output byte order is:
> - *             [0][1][2][3]-[4][5]-[6][7]-[8][9]-[10][11][12][13][14][15]
> - *           little endian output byte order is:
> - *             [3][2][1][0]-[5][4]-[7][6]-[8][9]-[10][11][12][13][14][15]
> - * - 'V' For a struct va_format which contains a format string * and va_list *,
> - *       call vsnprintf(->format, *->va_list).
> - *       Implements a "recursive vsnprintf".
> - *       Do not use this feature without some mechanism to verify the
> - *       correctness of the format string and va_list arguments.
> - * - 'K' For a kernel pointer that should be hidden from unprivileged users
> - * - 'NF' For a netdev_features_t
> - * - 'h[CDN]' For a variable-length buffer, it prints it as a hex string with
> - *            a certain separator (' ' by default):
> - *              C colon
> - *              D dash
> - *              N no separator
> - *            The maximum supported length is 64 bytes of the input. Consider
> - *            to use print_hex_dump() for the larger input.
> - * - 'a[pd]' For address types [p] phys_addr_t, [d] dma_addr_t and derivatives
> - *           (default assumed to be phys_addr_t, passed by reference)
> - * - 'd[234]' For a dentry name (optionally 2-4 last components)
> - * - 'D[234]' Same as 'd' but for a struct file
> - * - 'g' For block_device name (gendisk + partition number)
> - * - 'C' For a clock, it prints the name (Common Clock Framework) or address
> - *       (legacy clock framework) of the clock
> - * - 'Cn' For a clock, it prints the name (Common Clock Framework) or address
> - *        (legacy clock framework) of the clock
> - * - 'Cr' For a clock, it prints the current rate of the clock
> - * - 'G' For flags to be printed as a collection of symbolic strings that would
> - *       construct the specific value. Supported flags given by option:
> - *       p page flags (see struct page) given as pointer to unsigned long
> - *       g gfp flags (GFP_* and __GFP_*) given as pointer to gfp_t
> - *       v vma flags (VM_*) given as pointer to unsigned long
> - * - 'O' For a kobject based struct. Must be one of the following:
> - *       - 'OF[fnpPcCF]'  For a device tree object
> - *                        Without any optional arguments prints the full_name
> - *                        f device node full_name
> - *                        n device node name
> - *                        p device node phandle
> - *                        P device node path spec (name + @unit)
> - *                        F device node flags
> - *                        c major compatible string
> - *                        C full compatible string
> - *
> - * - 'x' For printing the address. Equivalent to "%lx".
> - *
> - * ** Please update also Documentation/printk-formats.txt when making changes **
> - *
>   * Note: The difference between 'S' and 'F' is that on ia64 and ppc64
>   * function pointers are really function descriptors, which contain a
>   * pointer to the real address.
> 

ta.
-- 
~Randy

Re: [PATCH] doc: convert printk-formats.txt to rst

[Jonathan Corbet]

On Wed,  6 Dec 2017 12:45:29 +1100
"Tobin C. Harding" <me@tobin.cc> wrote:

> Documentation/printk-formats.txt is a candidate for conversion to
> ReStructuredText format. Some effort has already been made to do this
> conversion even thought the suffix is currently .txt
> 
> Changes required to complete conversion
> 
> - Add double backticks where needed.
> - Add entry to Documentation/index.rst
> - Use flat-table instead of ASCII table.
> - Fix minor grammatical errors.
> - Capitalize headers and correctly order heading adornments.
> - Use 'Passed by reference' uniformly.
> - Update pointer documentation around %px specifier.
> - Fix erroneous double backticks (to commas).
> - Simplify documentation for kobject.
> - Convert lib/vsnprintf.c function docs to use kernel-docs and
>   include in Documentation/printk-formats.rst
> 
> Signed-off-by: Tobin C. Harding <me@tobin.cc>

Some comments from a quick review:

 - I would just put this into the core-api manual; we don't need to create
   a separate section for printk formats.

 - I agree with Markus and others about the table.  I think I would go a
   little further and encourage observance of the "use minimal markup"
   rule.  Lots of ``double backticks`` make for slightly nicer HTML/PDF
   output, but they come at the expense of plain-text readability, which
   is something we really don't want to sacrifice.

 - The vsprintf.c part is probably not for me to take, so it should be
   split out into a separate patch.

Thanks for working on this,

jon

Re: [PATCH] doc: convert printk-formats.txt to rst

[Tobin C. Harding]

On Wed, Dec 06, 2017 at 10:18:49AM -0800, Randy Dunlap wrote:

Thanks for your comments Randy.

> On 12/05/2017 05:45 PM, Tobin C. Harding wrote:
> > Documentation/printk-formats.txt is a candidate for conversion to
> > ReStructuredText format. Some effort has already been made to do this
> > conversion even thought the suffix is currently .txt
> > 
> > Changes required to complete conversion
> > 
> > - Add double backticks where needed.
> > - Add entry to Documentation/index.rst
> > - Use flat-table instead of ASCII table.
> > - Fix minor grammatical errors.
> > - Capitalize headers and correctly order heading adornments.
> 
> That's a style choice and an unneeded change (referring to Capitalize headers).

No worries, will revert.

> > - Use 'Passed by reference' uniformly.
> > - Update pointer documentation around %px specifier.
> > - Fix erroneous double backticks (to commas).
> > - Simplify documentation for kobject.
> > - Convert lib/vsnprintf.c function docs to use kernel-docs and
> >   include in Documentation/printk-formats.rst
> 
> good idea.
>
> > 
> > Signed-off-by: Tobin C. Harding <me@tobin.cc>
> > ---
> > 
> > The last two need special reviewing please. In particular the conversion
> > of kernel-docs in vsnprintf.c attempt was made to reduce documentation
> > duplication with comments in the source code being simplified in order
> > to be suitable for inclusion in Documentation/printk-formats.rst
> > 
> > I used -M when formatting the patch. If you don't like the diff with
> > this flag just holla.
> > 
> > thanks,
> > Tobin.
> > 
> >  Documentation/index.rst                            |  10 +
> >  .../{printk-formats.txt => printk-formats.rst}     | 295 ++++++++++++---------
> >  lib/vsprintf.c                                     | 160 +++++------
> >  3 files changed, 235 insertions(+), 230 deletions(-)
> >  rename Documentation/{printk-formats.txt => printk-formats.rst} (61%)
> 
> > diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.rst
> > similarity index 61%
> > rename from Documentation/printk-formats.txt
> > rename to Documentation/printk-formats.rst
> > index aa0a776c817a..51449d213748 100644
> > --- a/Documentation/printk-formats.txt
> > +++ b/Documentation/printk-formats.rst
> > @@ -1,6 +1,6 @@
> > -=========================================
> > -How to get printk format specifiers right
> > -=========================================
> > +=============================================
> > +How to Get ``printk`` Format Specifiers Right
> > +=============================================
> >  
> >  :Author: Randy Dunlap <rdunlap@infradead.org>
> >  :Author: Andrew Murray <amurray@mpc-data.co.uk>
> > @@ -8,56 +8,91 @@ How to get printk format specifiers right
> >  Integer types
> >  =============
> >  
> > -::
> > +For printing integer types, we have the following format specifiers:
> > +		
> > +   .. flat-table:: 
> > +      :widths: 2 2
> > +
> > +      * - **Type**
> > +	- **Specifier**
> > +
> > +      * - ``int``
> > +        - ``%d`` or ``%x``
> > +
> > +      * - ``unsigned int``
> > +	- ``%u`` or ``%x``
> > +
> > +      * - ``long``
> > +	- ``%ld`` or ``%lx``
> > +
> > +      * - ``unsigned long``
> > +	- ``%lu`` or ``%lx``
> > +
> > +      * - ``long long``
> > +	- ``%lld`` or ``%llx``
> >  
> > -	If variable is of Type,		use printk format specifier:
> > -	------------------------------------------------------------
> > -		int			%d or %x
> > -		unsigned int		%u or %x
> > -		long			%ld or %lx
> > -		unsigned long		%lu or %lx
> > -		long long		%lld or %llx
> > -		unsigned long long	%llu or %llx
> > -		size_t			%zu or %zx
> > -		ssize_t			%zd or %zx
> > -		s32			%d or %x
> > -		u32			%u or %x
> > -		s64			%lld or %llx
> > -		u64			%llu or %llx
> > -
> > -If <type> is dependent on a config option for its size (e.g., ``sector_t``,
> > +      * - ``unsigned long long``
> > +	- ``%llu`` or ``%llx``
> > +
> > +      * - ``size_t``
> > +	- ``%zu`` or ``%zx``
> > +
> > +      * - ``ssize_t``
> > +	- ``%zd`` or ``%zx``
> > +
> > +      * - ``s32``
> > +	- ``%d`` or ``%x``
> > +
> > +      * - ``u32``
> > +	- ``%u`` or ``%x``
> > +
> > +      * - ``s64``
> > +	- ``%lld`` or ``%llx``
> > +
> > +      * - ``u64``
> > +	- ``%llu`` or ``%llx``
> > +
> > +
> > +If ``<type>`` is dependent on a config option for its size (e.g., ``sector_t``,
> >  ``blkcnt_t``) or is architecture-dependent for its size (e.g., ``tcflag_t``),
> >  use a format specifier of its largest possible type and explicitly cast to it.
> >  
> >  Example::
> >  
> > -	printk("test: sector number/total blocks: %llu/%llu\n",
> > -		(unsigned long long)sector, (unsigned long long)blockcount);
> > +	printk("test: total blocks: %llu\n", (unsigned long long)blockcount);
> >  
> > -Reminder: ``sizeof()`` result is of type ``size_t``.
> > +Reminder: ``sizeof()`` returns type ``size_t``.
> >  
> > -The kernel's printf does not support ``%n``. For obvious reasons, floating
> > +The kernel's printf does not support ``%n``. For obvious reasons floating
> >  point formats (``%e, %f, %g, %a``) are also not recognized. Use of any
> >  unsupported specifier or length qualifier results in a WARN and early
> > -return from vsnprintf.
> > -
> > -Raw pointer value SHOULD be printed with %p. The kernel supports
> > -the following extended format specifiers for pointer types:
> > +return from ``vsnprintf()``.
> >  
> >  Pointer Types
> >  =============
> >  
> > -Pointers printed without a specifier extension (i.e unadorned %p) are
> > -hashed to give a unique identifier without leaking kernel addresses to user
> > -space. On 64 bit machines the first 32 bits are zeroed. If you _really_
> > -want the address see %px below.
> > +A raw pointer value may be printed with ``%p`` which will hash the address
> > +before printing. The Kernel also supports extended specifiers for printing
> > +pointers of different types.
> > +
> > +.. kernel-doc:: lib/vsprintf.c
> > +     :doc: Extended Format Pointer Specifiers
> > +
> > +
> > +Plain Pointers
> > +--------------
> >  
> >  ::
> >  
> >  	%p	abcdef12 or 00000000abcdef12
> >  
> > +Pointers printed without a specifier extension (i.e unadorned ``%p``) are
> > +hashed to give a unique identifier without leaking kernel addresses to user
> > +space. On 64 bit machines the first 32 bits are zeroed. If you *really*
> 
>              64-bit
> 
> > +want the address see ``%px`` below.
> > +
> >  Symbols/Function Pointers
> > -=========================
> > +-------------------------
> >  
> >  ::
> >  
> > @@ -69,61 +104,60 @@ Symbols/Function Pointers
> >  	%ps	versatile_init
> >  	%pB	prev_fn_of_versatile_init+0x88/0x88
> >  
> > -The ``F`` and ``f`` specifiers are for printing function pointers,
> > -for example, f->func, &gettimeofday. They have the same result as
> > -``S`` and ``s`` specifiers. But they do an extra conversion on
> > -ia64, ppc64 and parisc64 architectures where the function pointers
> > -are actually function descriptors.
> > +The ``F`` and ``f`` specifiers are for printing function pointers, for
> > +example, ``f->func``, ``&gettimeofday``. They have the same result as ``S``
> > +and ``s`` specifiers. But they do an extra conversion on ia64, ppc64 and
> > +parisc64 architectures where the function pointers are actually function
> > +descriptors.
> >  
> >  The ``S`` and ``s`` specifiers can be used for printing symbols
> > -from direct addresses, for example, __builtin_return_address(0),
> > -(void *)regs->ip. They result in the symbol name with (``S``) or
> > +from direct addresses, for example, ``__builtin_return_address(0)``,
> > +``(void *)regs->ip``. They result in the symbol name with (``S``) or
> >  without (``s``) offsets. If KALLSYMS are disabled then the symbol
> >  address is printed instead.
> >  
> >  The ``B`` specifier results in the symbol name with offsets and should be
> >  used when printing stack backtraces. The specifier takes into
> >  consideration the effect of compiler optimisations which may occur
> > -when tail-call``s are used and marked with the noreturn GCC attribute.
> > +when tail-call's are used and marked with the ``noreturn`` GCC attribute.
> >  
> >  Examples::
> >  
> >  	printk("Going to call: %pF\n", gettimeofday);
> >  	printk("Going to call: %pF\n", p->func);
> >  	printk("%s: called from %pS\n", __func__, (void *)_RET_IP_);
> > -	printk("%s: called from %pS\n", __func__,
> > -				(void *)__builtin_return_address(0));
> > +	printk("%s: called from %pS\n", __func__, (void *)__builtin_return_address(0));
> >  	printk("Faulted at %pS\n", (void *)regs->ip);
> >  	printk(" %s%pB\n", (reliable ? "" : "? "), (void *)*stack);
> >  
> >  Kernel Pointers
> > -===============
> > +---------------
> >  
> >  ::
> >  
> >  	%pK	01234567 or 0123456789abcdef
> >  
> >  For printing kernel pointers which should be hidden from unprivileged
> > -users. The behaviour of ``%pK`` depends on the ``kptr_restrict sysctl`` - see
> > -Documentation/sysctl/kernel.txt for more details.
> > +users. The behaviour of ``%pK`` depends on the ``kptr_restrict`` sysctl -
> > +see ``Documentation/sysctl/kernel.txt`` for more details.
> >  
> >  Unmodified Addresses
> > -====================
> > +--------------------
> >  
> >  ::
> >  
> >  	%px	01234567 or 0123456789abcdef
> >  
> > -For printing pointers when you _really_ want to print the address. Please
> > +For printing pointers when you *really* want to print the address. Please
> >  consider whether or not you are leaking sensitive information about the
> > -Kernel layout in memory before printing pointers with %px. %px is
> > -functionally equivalent to %lx. %px is preferred to %lx because it is more
> > -uniquely grep'able. If, in the future, we need to modify the way the Kernel
> > -handles printing pointers it will be nice to be able to find the call
> > -sites.
> > +kernel memory layout before printing pointers with ``%px``. ``%px`` is
> > +functionally equivalent to ``%lx`` (or ``%lu``). ``%px``, however, is
> > +preferable because it is more uniquely grep'able. If, in the future, we need
> > +to modify the way the Kernel handles printing pointers we will be better
> > +equipped to find the call sites.
> >  
> >  Struct Resources
> > -================
> > +----------------
> >  
> >  ::
> >  
> > @@ -132,12 +166,13 @@ Struct Resources
> >  	%pR	[mem 0x60000000-0x6fffffff pref] or
> >  		[mem 0x0000000060000000-0x000000006fffffff pref]
> >  
> > -For printing struct resources. The ``R`` and ``r`` specifiers result in a
> > +For printing ``struct resources``. The ``R`` and ``r`` specifiers result in a
> >  printed resource with (``R``) or without (``r``) a decoded flags member.
> > +
> >  Passed by reference.
> >  
> > -Physical addresses types ``phys_addr_t``
> > -========================================
> > +Physical Address Types ``phys_addr_t``
> > +--------------------------------------
> >  
> >  ::
> >  
> > @@ -145,20 +180,24 @@ Physical addresses types ``phys_addr_t``
> >  
> >  For printing a ``phys_addr_t`` type (and its derivatives, such as
> >  ``resource_size_t``) which can vary based on build options, regardless of
> > -the width of the CPU data path. Passed by reference.
> > +the width of the CPU data path.
> > +
> > +Passed by reference.
> >  
> > -DMA addresses types ``dma_addr_t``
> > -==================================
> > +DMA Address Types ``dma_addr_t``
> > +--------------------------------
> >  
> >  ::
> >  
> >  	%pad	0x01234567 or 0x0123456789abcdef
> >  
> >  For printing a ``dma_addr_t`` type which can vary based on build options,
> > -regardless of the width of the CPU data path. Passed by reference.
> > +regardless of the width of the CPU data path.
> >  
> > -Raw buffer as an escaped string
> > -===============================
> > +Passed by reference.
> > +
> > +Raw Buffer as an Escaped String
> > +-------------------------------
> >  
> >  ::
> >  
> > @@ -168,7 +207,7 @@ For printing raw buffer as an escaped string. For the following buffer::
> >  
> >  		1b 62 20 5c 43 07 22 90 0d 5d
> >  
> > -few examples show how the conversion would be done (the result string
> > +A few examples show how the conversion would be done (the result string
> >  without surrounding quotes)::
> >  
> >  		%*pE		"\eb \C\a"\220\r]"
> > @@ -194,8 +233,8 @@ printing SSIDs.
> >  
> >  If field width is omitted the 1 byte only will be escaped.
> 
>                              then
> I think...

Ha ha, I was borderline with this change when doing this patch. It may
not appear so but I did try to do the minimal amount of changes while
improving correctness. I appreciate your comments since hopefully I can
better make these judgment calls next time.

Will change as suggested.

> >  
> > -Raw buffer as a hex string
> > -==========================
> > +Raw Buffer as a Hex String
> > +--------------------------
> >  
> >  ::
> >  
> > @@ -205,11 +244,11 @@ Raw buffer as a hex string
> >  	%*phN	000102 ... 3f
> >  
> >  For printing a small buffers (up to 64 bytes long) as a hex string with
> > -certain separator. For the larger buffers consider to use
> > +certain separator. For the larger buffers consider using
> >  :c:func:`print_hex_dump`.
> >  
> > -MAC/FDDI addresses
> > -==================
> > +MAC/FDDI Addresses
> > +------------------
> >  
> >  ::
> >  
> > @@ -233,8 +272,8 @@ of Bluetooth addresses which are in the little endian order.
> >  
> >  Passed by reference.
> >  
> > -IPv4 addresses
> > -==============
> > +IPv4 Addresses
> > +--------------
> >  
> >  ::
> >  
> > @@ -252,8 +291,8 @@ no specifier is provided the default network/big endian order is used.
> >  
> >  Passed by reference.
> >  
> > -IPv6 addresses
> > -==============
> > +IPv6 Addresses
> > +--------------
> >  
> >  ::
> >  
> > @@ -271,8 +310,8 @@ http://tools.ietf.org/html/rfc5952
> >  
> >  Passed by reference.
> >  
> > -IPv4/IPv6 addresses (generic, with port, flowinfo, scope)
> > -=========================================================
> > +IPv4/IPv6 Addresses (generic, with port, flowinfo or scope)
> > +---------------------------------------------------------------
> 
> I prefer the additional (Oxford) comma.
> and why is the --- line longer than the header?

I had to look up Oxford comma. For the record, with Oxford comma would
be 

> > +IPv4/IPv6 Addresses (generic, with port, flowinfo, or scope)

After learning what that was I re investigated this one. I am now
leaning towards thinking the original is even better, although not
grammatically correct it better describes the code. 'or' is definitely
wrong because multiple [pfs] are allowed.

Will revert to original (and fix underscore, sloppy work :( )

> >  ::
> >  
> > @@ -282,8 +321,8 @@ IPv4/IPv6 addresses (generic, with port, flowinfo, scope)
> >  	%pISpc	1.2.3.4:12345	or [1:2:3:4:5:6:7:8]:12345
> >  	%p[Ii]S[pfschnbl]
> >  
> > -For printing an IP address without the need to distinguish whether it``s
> > -of type AF_INET or AF_INET6, a pointer to a valid ``struct sockaddr``,
> > +For printing an IP address without the need to distinguish whether it's
> > +of type AF_INET or AF_INET6. A pointer to a valid ``struct sockaddr``,
> >  specified through ``IS`` or ``iS``, can be passed to this format specifier.
> >  
> >  The additional ``p``, ``f``, and ``s`` specifiers are used to specify port
> > @@ -308,8 +347,8 @@ Further examples::
> >  	%pISsc		1.2.3.4		or [1:2:3:4:5:6:7:8]%1234567890
> >  	%pISpfc		1.2.3.4:12345	or [1:2:3:4:5:6:7:8]:12345/123456789
> >  
> > -UUID/GUID addresses
> > -===================
> > +UUID/GUID Addresses
> > +-------------------
> >  
> >  ::
> >  
> > @@ -318,18 +357,18 @@ UUID/GUID addresses
> >  	%pUl	03020100-0504-0706-0809-0a0b0c0e0e0f
> >  	%pUL	03020100-0504-0706-0809-0A0B0C0E0E0F
> >  
> > -For printing 16-byte UUID/GUIDs addresses. The additional 'l', 'L',
> > -'b' and 'B' specifiers are used to specify a little endian order in
> > -lower ('l') or upper case ('L') hex characters - and big endian order
> > -in lower ('b') or upper case ('B') hex characters.
> > +For printing 16-byte UUID/GUIDs addresses. The additional ``l``, ``L``,
> > +``b`` and ``B`` specifiers are used to specify a little endian order in
> > +lower (``l``) or upper case (``L``) hex digits - and big endian order
> > +in lower (``b``) or upper case (``B``) hex digits.
> >  
> >  Where no additional specifiers are used the default big endian
> > -order with lower case hex characters will be printed.
> > +order with lower case hex digits will be printed.
> 
> digits could imply base 10. but no big deal.

Are you sure about this? I was under the impression that when
representing a number the character set used are refereed to as 'digits'
irrespective of base.

hexadecimal digit
octal digit
digit (assumed base 10)

Open to correction though.

> >  
> >  Passed by reference.
> >  
> > -dentry names
> > -============
> > +Dentry Names
> > +------------
> >  
> >  ::
> >  
> > @@ -343,24 +382,24 @@ equivalent of ``%s`` ``dentry->d_name.name`` we used to use, ``%pd<n>`` prints
> >  
> >  Passed by reference.
> >  
> > -block_device names
> > -==================
> > +block_device Names
> > +------------------
> >  
> >  ::
> >  
> >  	%pg	sda, sda1 or loop0p1
> >  
> > -For printing name of block_device pointers.
> > +For printing name of ``block_device`` pointers.
> >  
> >  struct va_format
> > -================
> > +----------------
> >  
> >  ::
> >  
> >  	%pV
> >  
> > -For printing struct va_format structures. These contain a format string
> > -and va_list as follows::
> > +For printing ``struct va_format`` structures. These contain a format string
> > +and ``va_list`` as follows::
> >  
> >  	struct va_format {
> >  		const char *fmt;
> > @@ -370,36 +409,33 @@ and va_list as follows::
> >  Implements a "recursive vsnprintf".
> >  
> >  Do not use this feature without some mechanism to verify the
> > -correctness of the format string and va_list arguments.
> > +correctness of the format string and ``va_list`` arguments.
> >  
> >  Passed by reference.
> >  
> >  kobjects
> > -========
> > -
> > +--------
> > +	
> >  ::
> >  
> > -	%pO
> > +	%pOF[fnpPcCF]
> >  
> > -	Base specifier for kobject based structs. Must be followed with
> > -	character for specific type of kobject as listed below:
> >  
> > -	Device tree nodes:
> > +For printing kobject based structs (device nodes). Default behaviour is
> > +equivalent to ``%pOFf``.
> >  
> > -	%pOF[fnpPcCF]
> > +	- ``f`` device node full_name
> > +	- ``n`` device node name
> > +	- ``p`` device node phandle
> > +	- ``P`` device node path spec (name + @unit)
> > +	- ``F`` device node flags
> > +	- ``c`` major compatible string
> > +	- ``C`` full compatible string
> >  
> > -	For printing device tree nodes. The optional arguments are:
> > -	    f device node full_name
> > -	    n device node name
> > -	    p device node phandle
> > -	    P device node path spec (name + @unit)
> > -	    F device node flags
> > -	    c major compatible string
> > -	    C full compatible string
> > -	Without any arguments prints full_name (same as %pOFf)
> > -	The separator when using multiple arguments is ':'
> > +The separator when using multiple arguments is ``:``
> >  
> > -	Examples:
> > +Examples:
> > +::
> >  
> >  	%pOF	/foo/bar@0			- Node full name
> >  	%pOFf	/foo/bar@0			- Same as above
> > @@ -412,11 +448,10 @@ kobjects
> >  							P - Populated
> >  							B - Populated bus
> >  
> > -	Passed by reference.
> > -
> > +Passed by reference.
> >  
> >  struct clk
> > -==========
> > +----------
> >  
> >  ::
> >  
> > @@ -424,14 +459,14 @@ struct clk
> >  	%pCn	pll1
> >  	%pCr	1560000000
> >  
> > -For printing struct clk structures. ``%pC`` and ``%pCn`` print the name
> > +For printing ``struct clk structures``. ``%pC`` and ``%pCn`` print the name
> >  (Common Clock Framework) or address (legacy clock framework) of the
> >  structure; ``%pCr`` prints the current clock rate.
> >  
> >  Passed by reference.
> >  
> > -bitmap and its derivatives such as cpumask and nodemask
> > -=======================================================
> > +Bitmap and its Derivatives (such as cpumask and nodemask)
> > +---------------------------------------------------------
> >  
> >  ::
> >  
> > @@ -439,13 +474,13 @@ bitmap and its derivatives such as cpumask and nodemask
> >  	%*pbl	0,3-6,8-10
> >  
> >  For printing bitmap and its derivatives such as cpumask and nodemask,
> > -``%*pb`` output the bitmap with field width as the number of bits and ``%*pbl``
> > -output the bitmap as range list with field width as the number of bits.
> > +``%*pb`` outputs the bitmap with field width as the number of bits and ``%*pbl``
> > +outputs the bitmap as range list with field width as the number of bits.
> >  
> >  Passed by reference.
> >  
> > -Flags bitfields such as page flags, gfp_flags
> > -=============================================
> > +Flags Bitfields (such as page flags, gfp_flags)
> > +-----------------------------------------------
> >  
> >  ::
> >  
> > @@ -459,25 +494,27 @@ character. Currently supported are [p]age flags, [v]ma_flags (both
> >  expect ``unsigned long *``) and [g]fp_flags (expects ``gfp_t *``). The flag
> >  names and print order depends on the particular	type.
> >  
> > -Note that this format should not be used directly in :c:func:`TP_printk()` part
> > -of a tracepoint. Instead, use the ``show_*_flags()`` functions from
> > -<trace/events/mmflags.h>.
> > +Note that this format should not be used directly in the
> > +:c:func:`TP_printk()` part of a tracepoint. Instead, use the
> > +``show_*_flags()`` functions from ``<trace/events/mmflags.h>``. 
> >  
> >  Passed by reference.
> >  
> > -Network device features
> > -=======================
> > +Network Device Features
> > +-----------------------
> >  
> >  ::
> >  
> >  	%pNF	0x000000000000c000
> >  
> > -For printing netdev_features_t.
> > +For printing ``netdev_features_t``.
> >  
> >  Passed by reference.
> >  
> > -If you add other ``%p`` extensions, please extend lib/test_printf.c with
> > -one or more test cases, if at all feasible.
> > +Thanks
> > +======
> >  
> > +If you add other ``%p`` extensions, please extend ``lib/test_printf.c``
> > +with one or more test cases, if at all feasible.
> >  
> >  Thank you for your cooperation and attention.
> > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> > index 01c3957b2de6..f9247b78e8ef 100644
> > --- a/lib/vsprintf.c
> > +++ b/lib/vsprintf.c
> > @@ -1727,115 +1727,73 @@ static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec)
> >  	return number(buf, end, hashval, spec);
> >  }
> >  
> > +/**
> > + * DOC: Extended Format Pointer Specifiers
> > + *
> > + * Briefly we handle the following extensions:
> > + *
> > + * - ``F`` - For symbolic function descriptor pointers with offset.
> > + * - ``f`` - For simple symbolic function names without offset.
> > + *
> > + * - ``S`` - For symbolic direct pointers with offset.
> > + * - ``s`` - For symbolic direct pointers without offset.
> > + * - ``[FfSs]R`` - As above with ``__builtin_extract_return_addr()`` translation.
> > + * - ``B`` - For backtraced symbolic direct pointers with offset.
> > + * - ``R`` - For decoded struct resource, e.g., [mem 0x0-0x1f 64bit pref].
> > + * - ``r`` - For raw struct resource, e.g., [mem 0x0-0x1f flags 0x201].
> > + * - ``b[l]`` - For a bitmap, the number of bits is determined by the field
> > + *   width which must be explicitly specified either as part of the format
> > + *   string ``32b[l]`` or through ``*b[l]``, ``[l]`` selects range-list format
> > + *   instead of hex format.
> > + * - ``M`` - For a 6-byte MAC address, it prints the address in the usual
> > + *   colon-separated hex notation.
> > + * - ``m`` - For a 6-byte MAC address, it prints the hex address without colons.
> > + * - ``MF`` - For a 6-byte MAC FDDI address, it prints the address with a
> > + *   dash-separated hex notation.
> > + * - ``[mM]R`` - For a 6-byte MAC address, Reverse order (Bluetooth).
> > + * - ``I[46]`` - For IPv4/IPv6 addresses printed in the usual way.
> > + * - ``I[S][pfs]`` - For generic IPv4/IPv6 address (struct sockaddr *) that falls
> > + *   back to ``[4]`` or ``[6]`` and is able to print port ``[p]``,
> > + *   flowinfo ``[f]``, scope ``[s]``.
> > + * - ``i[46]`` - For 'raw' IPv4/IPv6 addresses IPv6 omits the colons (01020304...0f)
> > + *   IPv4 uses dot-separated decimal with leading 0's (010.123.045.006).
> > + * - ``i[S][pfs]`` - For generic IPv4/IPv6 address (struct sockaddr *) that falls back
> > + *   to ``[4]`` or ``[6]`` (``[pfs]`` as above).
> > + * - ``[Ii][4S][hnbl]`` - For IPv4 addresses in host, network, big or little endian order.
> > + * - ``I[6S]c`` - For IPv6 addresses printed as per http://tools.ietf.org/html/rfc5952.
> > + * - ``E[achnops]`` - For an escaped buffer.
> > + * - ``U`` - For a 16 byte UUID/GUID.
> > + * - ``V`` - For a ``struct va_format`` which contains a format ``string *``
> > + *   and ``va_list *``.
> > + * - ``K`` -  For a kernel pointer that should be hidden from unprivileged users.
> > + * - ``NF`` - For a ``netdev_features_t``.
> > + * - ``h[CDN]`` - For a variable-length buffer.
> > + * - ``a[pd]`` - For address types ``[p] phys_addr_t``, ``[d] dma_addr_t`` and
> > + *   derivatives.
> > + * - ``d[234]`` - For a dentry name (optionally 2-4 last components).
> > + * - ``D[234]`` - Same as 'd' but for a struct file.
> > + * - ``g`` - For ``block_device`` name (gendisk + partition number).
> > + * - ``C[n]`` - For a clock, it prints the name (Common Clock Framework) or
> > + *   address (legacy clock framework) of the clock. ``[n]`` is optional.
> > + * - ``Cr`` - For a clock, it prints the current rate of the clock.
> > + * - ``G`` - For flags to be printed as a collection of symbolic strings that
> > + *   would construct the specific value.
> > + * - ``O`` - For a kobject based struct (device node).
> > + * - ``x`` - For printing the address. Equivalent to ``%lx``.
> > + */
> > +
> >  /*
> >   * Show a '%p' thing.  A kernel extension is that the '%p' is followed
> >   * by an extra set of alphanumeric characters that are extended format
> >   * specifiers.
> >   *
> > + * Please see Documentation/printk-formats.rst for fuller description
> > + * of specifier extensions. Also please update this file when making
> 
> "this file" is the file that I am reading?  or could it be "that file"?

Got it.

thanks,
Tobin.

Re: [PATCH] doc: convert printk-formats.txt to rst

[Tobin C. Harding]

On Wed, Dec 06, 2017 at 11:23:25AM -0700, Jonathan Corbet wrote:
> On Wed,  6 Dec 2017 12:45:29 +1100
> "Tobin C. Harding" <me@tobin.cc> wrote:
> 
> > Documentation/printk-formats.txt is a candidate for conversion to
> > ReStructuredText format. Some effort has already been made to do this
> > conversion even thought the suffix is currently .txt
> > 
> > Changes required to complete conversion
> > 
> > - Add double backticks where needed.
> > - Add entry to Documentation/index.rst
> > - Use flat-table instead of ASCII table.
> > - Fix minor grammatical errors.
> > - Capitalize headers and correctly order heading adornments.
> > - Use 'Passed by reference' uniformly.
> > - Update pointer documentation around %px specifier.
> > - Fix erroneous double backticks (to commas).
> > - Simplify documentation for kobject.
> > - Convert lib/vsnprintf.c function docs to use kernel-docs and
> >   include in Documentation/printk-formats.rst
> > 
> > Signed-off-by: Tobin C. Harding <me@tobin.cc>
> 
> Some comments from a quick review:
> 
>  - I would just put this into the core-api manual; we don't need to create
>    a separate section for printk formats.

Cool, I was hoping you'd give some direction on this. thanks.

>  - I agree with Markus and others about the table.  I think I would go a
>    little further and encourage observance of the "use minimal markup"
>    rule.  Lots of ``double backticks`` make for slightly nicer HTML/PDF
>    output, but they come at the expense of plain-text readability, which
>    is something we really don't want to sacrifice.

Great. I personally don't read docs in HTML/PDF so I like this ruling.

>  - The vsprintf.c part is probably not for me to take, so it should be
>    split out into a separate patch.

I'm much less experienced than you Jon so please say if I am wrong but
since the rst file depends on the changes to vsprintf.c wouldn't it be
better if the changes went into the mainline together. I can split it
into a two patch set if that is cleaner but putting the two patches
through different trees seems like a bad idea because of the
dependency. For what it's worth, I don't believe lib/vsprintf.c has a
maintainer. Linus took changes to that file from my tree just
recently. I don't know how this stuff works though in regards to merge
conflicts. (Please take everything I say here with a pinch of salt since
I have only maintained a tree for a few weeks now.)

thanks,
Tobin.

Re: [PATCH] doc: convert printk-formats.txt to rst

[Tobin C. Harding]

On Wed, Dec 06, 2017 at 10:18:49AM -0800, Randy Dunlap wrote:
> On 12/05/2017 05:45 PM, Tobin C. Harding wrote:
> > Documentation/printk-formats.txt is a candidate for conversion to
> > ReStructuredText format. Some effort has already been made to do this
> > conversion even thought the suffix is currently .txt
> > 
> > Changes required to complete conversion
> > 
> > - Add double backticks where needed.
> > - Add entry to Documentation/index.rst
> > - Use flat-table instead of ASCII table.
> > - Fix minor grammatical errors.
> > - Capitalize headers and correctly order heading adornments.
> 
> That's a style choice and an unneeded change (referring to Capitalize headers).
> 
> > - Use 'Passed by reference' uniformly.
> > - Update pointer documentation around %px specifier.
> > - Fix erroneous double backticks (to commas).
> > - Simplify documentation for kobject.
> > - Convert lib/vsnprintf.c function docs to use kernel-docs and
> >   include in Documentation/printk-formats.rst
> 
> good idea.
> 
> > 
> > Signed-off-by: Tobin C. Harding <me@tobin.cc>
> > ---
> > 
> > The last two need special reviewing please. In particular the conversion
> > of kernel-docs in vsnprintf.c attempt was made to reduce documentation
> > duplication with comments in the source code being simplified in order
> > to be suitable for inclusion in Documentation/printk-formats.rst
> > 
> > I used -M when formatting the patch. If you don't like the diff with
> > this flag just holla.
> > 
> > thanks,
> > Tobin.
> > 
> >  Documentation/index.rst                            |  10 +
> >  .../{printk-formats.txt => printk-formats.rst}     | 295 ++++++++++++---------
> >  lib/vsprintf.c                                     | 160 +++++------
> >  3 files changed, 235 insertions(+), 230 deletions(-)
> >  rename Documentation/{printk-formats.txt => printk-formats.rst} (61%)
> 
> > diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.rst
> > similarity index 61%
> > rename from Documentation/printk-formats.txt
> > rename to Documentation/printk-formats.rst
> > index aa0a776c817a..51449d213748 100644
> > --- a/Documentation/printk-formats.txt
> > +++ b/Documentation/printk-formats.rst
> > @@ -1,6 +1,6 @@
> > -=========================================
> > -How to get printk format specifiers right
> > -=========================================
> > +=============================================
> > +How to Get ``printk`` Format Specifiers Right
> > +=============================================
> >  
> >  :Author: Randy Dunlap <rdunlap@infradead.org>
> >  :Author: Andrew Murray <amurray@mpc-data.co.uk>
> > @@ -8,56 +8,91 @@ How to get printk format specifiers right
> >  Integer types
> >  =============
> >  
> > -::
> > +For printing integer types, we have the following format specifiers:
> > +		
> > +   .. flat-table:: 
> > +      :widths: 2 2
> > +
> > +      * - **Type**
> > +	- **Specifier**
> > +
> > +      * - ``int``
> > +        - ``%d`` or ``%x``
> > +
> > +      * - ``unsigned int``
> > +	- ``%u`` or ``%x``
> > +
> > +      * - ``long``
> > +	- ``%ld`` or ``%lx``
> > +
> > +      * - ``unsigned long``
> > +	- ``%lu`` or ``%lx``
> > +
> > +      * - ``long long``
> > +	- ``%lld`` or ``%llx``
> >  
> > -	If variable is of Type,		use printk format specifier:
> > -	------------------------------------------------------------
> > -		int			%d or %x
> > -		unsigned int		%u or %x
> > -		long			%ld or %lx
> > -		unsigned long		%lu or %lx
> > -		long long		%lld or %llx
> > -		unsigned long long	%llu or %llx
> > -		size_t			%zu or %zx
> > -		ssize_t			%zd or %zx
> > -		s32			%d or %x
> > -		u32			%u or %x
> > -		s64			%lld or %llx
> > -		u64			%llu or %llx
> > -
> > -If <type> is dependent on a config option for its size (e.g., ``sector_t``,
> > +      * - ``unsigned long long``
> > +	- ``%llu`` or ``%llx``
> > +
> > +      * - ``size_t``
> > +	- ``%zu`` or ``%zx``
> > +
> > +      * - ``ssize_t``
> > +	- ``%zd`` or ``%zx``
> > +
> > +      * - ``s32``
> > +	- ``%d`` or ``%x``
> > +
> > +      * - ``u32``
> > +	- ``%u`` or ``%x``
> > +
> > +      * - ``s64``
> > +	- ``%lld`` or ``%llx``
> > +
> > +      * - ``u64``
> > +	- ``%llu`` or ``%llx``
> > +
> > +
> > +If ``<type>`` is dependent on a config option for its size (e.g., ``sector_t``,
> >  ``blkcnt_t``) or is architecture-dependent for its size (e.g., ``tcflag_t``),
> >  use a format specifier of its largest possible type and explicitly cast to it.
> >  
> >  Example::
> >  
> > -	printk("test: sector number/total blocks: %llu/%llu\n",
> > -		(unsigned long long)sector, (unsigned long long)blockcount);
> > +	printk("test: total blocks: %llu\n", (unsigned long long)blockcount);
> >  
> > -Reminder: ``sizeof()`` result is of type ``size_t``.
> > +Reminder: ``sizeof()`` returns type ``size_t``.
> >  
> > -The kernel's printf does not support ``%n``. For obvious reasons, floating
> > +The kernel's printf does not support ``%n``. For obvious reasons floating
> >  point formats (``%e, %f, %g, %a``) are also not recognized. Use of any
> >  unsupported specifier or length qualifier results in a WARN and early
> > -return from vsnprintf.
> > -
> > -Raw pointer value SHOULD be printed with %p. The kernel supports
> > -the following extended format specifiers for pointer types:
> > +return from ``vsnprintf()``.
> >  
> >  Pointer Types
> >  =============
> >  
> > -Pointers printed without a specifier extension (i.e unadorned %p) are
> > -hashed to give a unique identifier without leaking kernel addresses to user
> > -space. On 64 bit machines the first 32 bits are zeroed. If you _really_
> > -want the address see %px below.
> > +A raw pointer value may be printed with ``%p`` which will hash the address
> > +before printing. The Kernel also supports extended specifiers for printing
> > +pointers of different types.
> > +
> > +.. kernel-doc:: lib/vsprintf.c
> > +     :doc: Extended Format Pointer Specifiers
> > +
> > +
> > +Plain Pointers
> > +--------------
> >  
> >  ::
> >  
> >  	%p	abcdef12 or 00000000abcdef12
> >  
> > +Pointers printed without a specifier extension (i.e unadorned ``%p``) are
> > +hashed to give a unique identifier without leaking kernel addresses to user
> > +space. On 64 bit machines the first 32 bits are zeroed. If you *really*
> 
>              64-bit
> 
> > +want the address see ``%px`` below.
> > +
> >  Symbols/Function Pointers
> > -=========================
> > +-------------------------
> >  
> >  ::
> >  
> > @@ -69,61 +104,60 @@ Symbols/Function Pointers
> >  	%ps	versatile_init
> >  	%pB	prev_fn_of_versatile_init+0x88/0x88
> >  
> > -The ``F`` and ``f`` specifiers are for printing function pointers,
> > -for example, f->func, &gettimeofday. They have the same result as
> > -``S`` and ``s`` specifiers. But they do an extra conversion on
> > -ia64, ppc64 and parisc64 architectures where the function pointers
> > -are actually function descriptors.
> > +The ``F`` and ``f`` specifiers are for printing function pointers, for
> > +example, ``f->func``, ``&gettimeofday``. They have the same result as ``S``
> > +and ``s`` specifiers. But they do an extra conversion on ia64, ppc64 and
> > +parisc64 architectures where the function pointers are actually function
> > +descriptors.
> >  
> >  The ``S`` and ``s`` specifiers can be used for printing symbols
> > -from direct addresses, for example, __builtin_return_address(0),
> > -(void *)regs->ip. They result in the symbol name with (``S``) or
> > +from direct addresses, for example, ``__builtin_return_address(0)``,
> > +``(void *)regs->ip``. They result in the symbol name with (``S``) or
> >  without (``s``) offsets. If KALLSYMS are disabled then the symbol
> >  address is printed instead.
> >  
> >  The ``B`` specifier results in the symbol name with offsets and should be
> >  used when printing stack backtraces. The specifier takes into
> >  consideration the effect of compiler optimisations which may occur
> > -when tail-call``s are used and marked with the noreturn GCC attribute.
> > +when tail-call's are used and marked with the ``noreturn`` GCC attribute.
> >  
> >  Examples::
> >  
> >  	printk("Going to call: %pF\n", gettimeofday);
> >  	printk("Going to call: %pF\n", p->func);
> >  	printk("%s: called from %pS\n", __func__, (void *)_RET_IP_);
> > -	printk("%s: called from %pS\n", __func__,
> > -				(void *)__builtin_return_address(0));
> > +	printk("%s: called from %pS\n", __func__, (void *)__builtin_return_address(0));
> >  	printk("Faulted at %pS\n", (void *)regs->ip);
> >  	printk(" %s%pB\n", (reliable ? "" : "? "), (void *)*stack);
> >  
> >  Kernel Pointers
> > -===============
> > +---------------
> >  
> >  ::
> >  
> >  	%pK	01234567 or 0123456789abcdef
> >  
> >  For printing kernel pointers which should be hidden from unprivileged
> > -users. The behaviour of ``%pK`` depends on the ``kptr_restrict sysctl`` - see
> > -Documentation/sysctl/kernel.txt for more details.
> > +users. The behaviour of ``%pK`` depends on the ``kptr_restrict`` sysctl -
> > +see ``Documentation/sysctl/kernel.txt`` for more details.
> >  
> >  Unmodified Addresses
> > -====================
> > +--------------------
> >  
> >  ::
> >  
> >  	%px	01234567 or 0123456789abcdef
> >  
> > -For printing pointers when you _really_ want to print the address. Please
> > +For printing pointers when you *really* want to print the address. Please
> >  consider whether or not you are leaking sensitive information about the
> > -Kernel layout in memory before printing pointers with %px. %px is
> > -functionally equivalent to %lx. %px is preferred to %lx because it is more
> > -uniquely grep'able. If, in the future, we need to modify the way the Kernel
> > -handles printing pointers it will be nice to be able to find the call
> > -sites.
> > +kernel memory layout before printing pointers with ``%px``. ``%px`` is
> > +functionally equivalent to ``%lx`` (or ``%lu``). ``%px``, however, is
> > +preferable because it is more uniquely grep'able. If, in the future, we need
> > +to modify the way the Kernel handles printing pointers we will be better
> > +equipped to find the call sites.
> >  
> >  Struct Resources
> > -================
> > +----------------
> >  
> >  ::
> >  
> > @@ -132,12 +166,13 @@ Struct Resources
> >  	%pR	[mem 0x60000000-0x6fffffff pref] or
> >  		[mem 0x0000000060000000-0x000000006fffffff pref]
> >  
> > -For printing struct resources. The ``R`` and ``r`` specifiers result in a
> > +For printing ``struct resources``. The ``R`` and ``r`` specifiers result in a
> >  printed resource with (``R``) or without (``r``) a decoded flags member.
> > +
> >  Passed by reference.
> >  
> > -Physical addresses types ``phys_addr_t``
> > -========================================
> > +Physical Address Types ``phys_addr_t``
> > +--------------------------------------
> >  
> >  ::
> >  
> > @@ -145,20 +180,24 @@ Physical addresses types ``phys_addr_t``
> >  
> >  For printing a ``phys_addr_t`` type (and its derivatives, such as
> >  ``resource_size_t``) which can vary based on build options, regardless of
> > -the width of the CPU data path. Passed by reference.
> > +the width of the CPU data path.
> > +
> > +Passed by reference.
> >  
> > -DMA addresses types ``dma_addr_t``
> > -==================================
> > +DMA Address Types ``dma_addr_t``
> > +--------------------------------
> >  
> >  ::
> >  
> >  	%pad	0x01234567 or 0x0123456789abcdef
> >  
> >  For printing a ``dma_addr_t`` type which can vary based on build options,
> > -regardless of the width of the CPU data path. Passed by reference.
> > +regardless of the width of the CPU data path.
> >  
> > -Raw buffer as an escaped string
> > -===============================
> > +Passed by reference.
> > +
> > +Raw Buffer as an Escaped String
> > +-------------------------------
> >  
> >  ::
> >  
> > @@ -168,7 +207,7 @@ For printing raw buffer as an escaped string. For the following buffer::
> >  
> >  		1b 62 20 5c 43 07 22 90 0d 5d
> >  
> > -few examples show how the conversion would be done (the result string
> > +A few examples show how the conversion would be done (the result string
> >  without surrounding quotes)::
> >  
> >  		%*pE		"\eb \C\a"\220\r]"
> > @@ -194,8 +233,8 @@ printing SSIDs.
> >  
> >  If field width is omitted the 1 byte only will be escaped.
> 
>                              then
> I think...
> 
> >  
> > -Raw buffer as a hex string
> > -==========================
> > +Raw Buffer as a Hex String
> > +--------------------------
> >  
> >  ::
> >  
> > @@ -205,11 +244,11 @@ Raw buffer as a hex string
> >  	%*phN	000102 ... 3f
> >  
> >  For printing a small buffers (up to 64 bytes long) as a hex string with
> > -certain separator. For the larger buffers consider to use
> > +certain separator. For the larger buffers consider using
> >  :c:func:`print_hex_dump`.
> >  
> > -MAC/FDDI addresses
> > -==================
> > +MAC/FDDI Addresses
> > +------------------
> >  
> >  ::
> >  
> > @@ -233,8 +272,8 @@ of Bluetooth addresses which are in the little endian order.
> >  
> >  Passed by reference.
> >  
> > -IPv4 addresses
> > -==============
> > +IPv4 Addresses
> > +--------------
> >  
> >  ::
> >  
> > @@ -252,8 +291,8 @@ no specifier is provided the default network/big endian order is used.
> >  
> >  Passed by reference.
> >  
> > -IPv6 addresses
> > -==============
> > +IPv6 Addresses
> > +--------------
> >  
> >  ::
> >  
> > @@ -271,8 +310,8 @@ http://tools.ietf.org/html/rfc5952
> >  
> >  Passed by reference.
> >  
> > -IPv4/IPv6 addresses (generic, with port, flowinfo, scope)
> > -=========================================================
> > +IPv4/IPv6 Addresses (generic, with port, flowinfo or scope)
> > +---------------------------------------------------------------
> 
> I prefer the additional (Oxford) comma.
> and why is the --- line longer than the header?
> 
> >  
> >  ::
> >  
> > @@ -282,8 +321,8 @@ IPv4/IPv6 addresses (generic, with port, flowinfo, scope)
> >  	%pISpc	1.2.3.4:12345	or [1:2:3:4:5:6:7:8]:12345
> >  	%p[Ii]S[pfschnbl]
> >  
> > -For printing an IP address without the need to distinguish whether it``s
> > -of type AF_INET or AF_INET6, a pointer to a valid ``struct sockaddr``,
> > +For printing an IP address without the need to distinguish whether it's
> > +of type AF_INET or AF_INET6. A pointer to a valid ``struct sockaddr``,
> >  specified through ``IS`` or ``iS``, can be passed to this format specifier.
> >  
> >  The additional ``p``, ``f``, and ``s`` specifiers are used to specify port
> > @@ -308,8 +347,8 @@ Further examples::
> >  	%pISsc		1.2.3.4		or [1:2:3:4:5:6:7:8]%1234567890
> >  	%pISpfc		1.2.3.4:12345	or [1:2:3:4:5:6:7:8]:12345/123456789
> >  
> > -UUID/GUID addresses
> > -===================
> > +UUID/GUID Addresses
> > +-------------------
> >  
> >  ::
> >  
> > @@ -318,18 +357,18 @@ UUID/GUID addresses
> >  	%pUl	03020100-0504-0706-0809-0a0b0c0e0e0f
> >  	%pUL	03020100-0504-0706-0809-0A0B0C0E0E0F
> >  
> > -For printing 16-byte UUID/GUIDs addresses. The additional 'l', 'L',
> > -'b' and 'B' specifiers are used to specify a little endian order in
> > -lower ('l') or upper case ('L') hex characters - and big endian order
> > -in lower ('b') or upper case ('B') hex characters.
> > +For printing 16-byte UUID/GUIDs addresses. The additional ``l``, ``L``,
> > +``b`` and ``B`` specifiers are used to specify a little endian order in
> > +lower (``l``) or upper case (``L``) hex digits - and big endian order
> > +in lower (``b``) or upper case (``B``) hex digits.
> >  
> >  Where no additional specifiers are used the default big endian
> > -order with lower case hex characters will be printed.
> > +order with lower case hex digits will be printed.
> 
> digits could imply base 10. but no big deal.

Another place in the file uses 'hex notation'. I guess we should unify
all usage (within this file at least).

thanks,
Tobin.

Re: [PATCH] doc: convert printk-formats.txt to rst

[Randy Dunlap]

On 12/06/2017 01:16 PM, Tobin C. Harding wrote:
> On Wed, Dec 06, 2017 at 10:18:49AM -0800, Randy Dunlap wrote:
> 
> Thanks for your comments Randy.
> 

>>>  Documentation/index.rst                            |  10 +
>>>  .../{printk-formats.txt => printk-formats.rst}     | 295 ++++++++++++---------
>>>  lib/vsprintf.c                                     | 160 +++++------
>>>  3 files changed, 235 insertions(+), 230 deletions(-)
>>>  rename Documentation/{printk-formats.txt => printk-formats.rst} (61%)
>>
>>> diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.rst
>>> similarity index 61%
>>> rename from Documentation/printk-formats.txt
>>> rename to Documentation/printk-formats.rst
>>> index aa0a776c817a..51449d213748 100644
>>> --- a/Documentation/printk-formats.txt
>>> +++ b/Documentation/printk-formats.rst

>>> @@ -194,8 +233,8 @@ printing SSIDs.
>>>  
>>>  If field width is omitted the 1 byte only will be escaped.
>>
>>                              then
>> I think...
> 
> Ha ha, I was borderline with this change when doing this patch. It may
> not appear so but I did try to do the minimal amount of changes while
> improving correctness. I appreciate your comments since hopefully I can
> better make these judgment calls next time.

I wasn't so sure about that attempt (at minimal changes).  :)

> Will change as suggested.

>>>  Where no additional specifiers are used the default big endian
>>> -order with lower case hex characters will be printed.
>>> +order with lower case hex digits will be printed.
>>
>> digits could imply base 10. but no big deal.
> 
> Are you sure about this? I was under the impression that when
> representing a number the character set used are refereed to as 'digits'
> irrespective of base.
> 
> hexadecimal digit
> octal digit
> digit (assumed base 10)
> 
> Open to correction though.

Like I said, I don't care strongly about this. (I'm easy.)
but hex notation (like you said later) sounds good.


-- 
~Randy

Re: [PATCH] doc: convert printk-formats.txt to rst

[Tobin C. Harding]

On Wed, Dec 06, 2017 at 04:39:58PM -0800, Randy Dunlap wrote:
> On 12/06/2017 01:16 PM, Tobin C. Harding wrote:
> > On Wed, Dec 06, 2017 at 10:18:49AM -0800, Randy Dunlap wrote:
> > 
> > Thanks for your comments Randy.
> > 
> 
> >>>  Documentation/index.rst                            |  10 +
> >>>  .../{printk-formats.txt => printk-formats.rst}     | 295 ++++++++++++---------
> >>>  lib/vsprintf.c                                     | 160 +++++------
> >>>  3 files changed, 235 insertions(+), 230 deletions(-)
> >>>  rename Documentation/{printk-formats.txt => printk-formats.rst} (61%)
> >>
> >>> diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.rst
> >>> similarity index 61%
> >>> rename from Documentation/printk-formats.txt
> >>> rename to Documentation/printk-formats.rst
> >>> index aa0a776c817a..51449d213748 100644
> >>> --- a/Documentation/printk-formats.txt
> >>> +++ b/Documentation/printk-formats.rst
> 
> >>> @@ -194,8 +233,8 @@ printing SSIDs.
> >>>  
> >>>  If field width is omitted the 1 byte only will be escaped.
> >>
> >>                              then
> >> I think...
> > 
> > Ha ha, I was borderline with this change when doing this patch. It may
> > not appear so but I did try to do the minimal amount of changes while
> > improving correctness. I appreciate your comments since hopefully I can
> > better make these judgment calls next time.
> 
> I wasn't so sure about that attempt (at minimal changes).  :)

ROFL

> > Will change as suggested.
> 
> >>>  Where no additional specifiers are used the default big endian
> >>> -order with lower case hex characters will be printed.
> >>> +order with lower case hex digits will be printed.
> >>
> >> digits could imply base 10. but no big deal.
> > 
> > Are you sure about this? I was under the impression that when
> > representing a number the character set used are refereed to as 'digits'
> > irrespective of base.
> > 
> > hexadecimal digit
> > octal digit
> > digit (assumed base 10)
> > 
> > Open to correction though.
> 
> Like I said, I don't care strongly about this. (I'm easy.)
> but hex notation (like you said later) sounds good.

Got it.

thanks,
Tobin.

Re: [PATCH] doc: convert printk-formats.txt to rst

[Kees Cook]

On Tue, Dec 5, 2017 at 5:45 PM, Tobin C. Harding <me@tobin.cc> wrote:
> Documentation/printk-formats.txt is a candidate for conversion to
> ReStructuredText format. Some effort has already been made to do this
> conversion even thought the suffix is currently .txt
>
> Changes required to complete conversion
>
> - Add double backticks where needed.
> - Add entry to Documentation/index.rst
> - Use flat-table instead of ASCII table.
> - Fix minor grammatical errors.
> - Capitalize headers and correctly order heading adornments.
> - Use 'Passed by reference' uniformly.
> - Update pointer documentation around %px specifier.
> - Fix erroneous double backticks (to commas).
> - Simplify documentation for kobject.
> - Convert lib/vsnprintf.c function docs to use kernel-docs and
>   include in Documentation/printk-formats.rst

Awesome!

>
> Signed-off-by: Tobin C. Harding <me@tobin.cc>
> ---
>
> The last two need special reviewing please. In particular the conversion
> of kernel-docs in vsnprintf.c attempt was made to reduce documentation
> duplication with comments in the source code being simplified in order
> to be suitable for inclusion in Documentation/printk-formats.rst
>
> I used -M when formatting the patch. If you don't like the diff with
> this flag just holla.
>
> thanks,
> Tobin.
>
>  Documentation/index.rst                            |  10 +
>  .../{printk-formats.txt => printk-formats.rst}     | 295 ++++++++++++---------
>  lib/vsprintf.c                                     | 160 +++++------
>  3 files changed, 235 insertions(+), 230 deletions(-)
>  rename Documentation/{printk-formats.txt => printk-formats.rst} (61%)
>
> diff --git a/Documentation/index.rst b/Documentation/index.rst
> index cb7f1ba5b3b1..83ace60efbe7 100644
> --- a/Documentation/index.rst
> +++ b/Documentation/index.rst
> @@ -87,6 +87,16 @@ implementation.
>
>     sh/index
>
> +Miscellaneous documentation
> +---------------------------
> +
> +These guides contain general information useful when writing kernel code.
> +
> +.. toctree::
> +   :maxdepth: 1
> +
> +   printk-formats

I actually think this belongs in the kernel core API documentation
list (Documentation/core-api/index.rst). I defer to Jon's opinion,
though.

> [...]
> +.. kernel-doc:: lib/vsprintf.c
> +     :doc: Extended Format Pointer Specifiers

Awesome!

> [...]
> +For printing pointers when you *really* want to print the address. Please
>  consider whether or not you are leaking sensitive information about the
> -Kernel layout in memory before printing pointers with %px. %px is
> -functionally equivalent to %lx. %px is preferred to %lx because it is more
> -uniquely grep'able. If, in the future, we need to modify the way the Kernel
> -handles printing pointers it will be nice to be able to find the call
> -sites.
> +kernel memory layout before printing pointers with ``%px``. ``%px`` is
> +functionally equivalent to ``%lx`` (or ``%lu``). ``%px``, however, is
> +preferable because it is more uniquely grep'able. If, in the future, we need
> +to modify the way the Kernel handles printing pointers we will be better
> +equipped to find the call sites.

nit? You de-capitalized Kernel at the start but not at the end. "the
Kernel handles ..."

> [...]
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 01c3957b2de6..f9247b78e8ef 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1727,115 +1727,73 @@ static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec)
>         return number(buf, end, hashval, spec);
>  }
>
> +/**
> + * DOC: Extended Format Pointer Specifiers
> + *
> + * Briefly we handle the following extensions:
> + *
> + * - ``F`` - For symbolic function descriptor pointers with offset.
> + * - ``f`` - For simple symbolic function names without offset.
> + *
> + * - ``S`` - For symbolic direct pointers with offset.
> + * - ``s`` - For symbolic direct pointers without offset.
> + * - ``[FfSs]R`` - As above with ``__builtin_extract_return_addr()`` translation.
> + * - ``B`` - For backtraced symbolic direct pointers with offset.
> + * - ``R`` - For decoded struct resource, e.g., [mem 0x0-0x1f 64bit pref].
> + * - ``r`` - For raw struct resource, e.g., [mem 0x0-0x1f flags 0x201].
> + * - ``b[l]`` - For a bitmap, the number of bits is determined by the field
> + *   width which must be explicitly specified either as part of the format
> + *   string ``32b[l]`` or through ``*b[l]``, ``[l]`` selects range-list format
> + *   instead of hex format.
> + * - ``M`` - For a 6-byte MAC address, it prints the address in the usual
> + *   colon-separated hex notation.
> + * - ``m`` - For a 6-byte MAC address, it prints the hex address without colons.
> + * - ``MF`` - For a 6-byte MAC FDDI address, it prints the address with a
> + *   dash-separated hex notation.
> + * - ``[mM]R`` - For a 6-byte MAC address, Reverse order (Bluetooth).
> + * - ``I[46]`` - For IPv4/IPv6 addresses printed in the usual way.
> + * - ``I[S][pfs]`` - For generic IPv4/IPv6 address (struct sockaddr *) that falls
> + *   back to ``[4]`` or ``[6]`` and is able to print port ``[p]``,
> + *   flowinfo ``[f]``, scope ``[s]``.
> + * - ``i[46]`` - For 'raw' IPv4/IPv6 addresses IPv6 omits the colons (01020304...0f)
> + *   IPv4 uses dot-separated decimal with leading 0's (010.123.045.006).
> + * - ``i[S][pfs]`` - For generic IPv4/IPv6 address (struct sockaddr *) that falls back
> + *   to ``[4]`` or ``[6]`` (``[pfs]`` as above).
> + * - ``[Ii][4S][hnbl]`` - For IPv4 addresses in host, network, big or little endian order.
> + * - ``I[6S]c`` - For IPv6 addresses printed as per http://tools.ietf.org/html/rfc5952.
> + * - ``E[achnops]`` - For an escaped buffer.
> + * - ``U`` - For a 16 byte UUID/GUID.
> + * - ``V`` - For a ``struct va_format`` which contains a format ``string *``
> + *   and ``va_list *``.
> + * - ``K`` -  For a kernel pointer that should be hidden from unprivileged users.
> + * - ``NF`` - For a ``netdev_features_t``.
> + * - ``h[CDN]`` - For a variable-length buffer.
> + * - ``a[pd]`` - For address types ``[p] phys_addr_t``, ``[d] dma_addr_t`` and
> + *   derivatives.
> + * - ``d[234]`` - For a dentry name (optionally 2-4 last components).
> + * - ``D[234]`` - Same as 'd' but for a struct file.
> + * - ``g`` - For ``block_device`` name (gendisk + partition number).
> + * - ``C[n]`` - For a clock, it prints the name (Common Clock Framework) or
> + *   address (legacy clock framework) of the clock. ``[n]`` is optional.
> + * - ``Cr`` - For a clock, it prints the current rate of the clock.
> + * - ``G`` - For flags to be printed as a collection of symbolic strings that
> + *   would construct the specific value.
> + * - ``O`` - For a kobject based struct (device node).
> + * - ``x`` - For printing the address. Equivalent to ``%lx``.
> + */
> +
>  /*
>   * Show a '%p' thing.  A kernel extension is that the '%p' is followed
>   * by an extra set of alphanumeric characters that are extended format
>   * specifiers.
>   *
> + * Please see Documentation/printk-formats.rst for fuller description
> + * of specifier extensions. Also please update this file when making
> + * changes.

Perhaps "... please update the kernel-doc above when making changes to
this file." ?

> + *
>   * Please update scripts/checkpatch.pl when adding/removing conversion
>   * characters.  (Search for "check for vsprintf extension").
>   *
> - * Right now we handle:
> - *
> - * - 'F' For symbolic function descriptor pointers with offset
> - * - 'f' For simple symbolic function names without offset
> - * - 'S' For symbolic direct pointers with offset
> - * - 's' For symbolic direct pointers without offset
> - * - '[FfSs]R' as above with __builtin_extract_return_addr() translation
> - * - 'B' For backtraced symbolic direct pointers with offset
> - * - 'R' For decoded struct resource, e.g., [mem 0x0-0x1f 64bit pref]
> - * - 'r' For raw struct resource, e.g., [mem 0x0-0x1f flags 0x201]
> - * - 'b[l]' For a bitmap, the number of bits is determined by the field
> - *       width which must be explicitly specified either as part of the
> - *       format string '%32b[l]' or through '%*b[l]', [l] selects
> - *       range-list format instead of hex format
> - * - 'M' For a 6-byte MAC address, it prints the address in the
> - *       usual colon-separated hex notation
> - * - 'm' For a 6-byte MAC address, it prints the hex address without colons
> - * - 'MF' For a 6-byte MAC FDDI address, it prints the address
> - *       with a dash-separated hex notation
> - * - '[mM]R' For a 6-byte MAC address, Reverse order (Bluetooth)
> - * - 'I' [46] for IPv4/IPv6 addresses printed in the usual way
> - *       IPv4 uses dot-separated decimal without leading 0's (1.2.3.4)
> - *       IPv6 uses colon separated network-order 16 bit hex with leading 0's
> - *       [S][pfs]
> - *       Generic IPv4/IPv6 address (struct sockaddr *) that falls back to
> - *       [4] or [6] and is able to print port [p], flowinfo [f], scope [s]
> - * - 'i' [46] for 'raw' IPv4/IPv6 addresses
> - *       IPv6 omits the colons (01020304...0f)
> - *       IPv4 uses dot-separated decimal with leading 0's (010.123.045.006)
> - *       [S][pfs]
> - *       Generic IPv4/IPv6 address (struct sockaddr *) that falls back to
> - *       [4] or [6] and is able to print port [p], flowinfo [f], scope [s]
> - * - '[Ii][4S][hnbl]' IPv4 addresses in host, network, big or little endian order
> - * - 'I[6S]c' for IPv6 addresses printed as specified by
> - *       http://tools.ietf.org/html/rfc5952
> - * - 'E[achnops]' For an escaped buffer, where rules are defined by combination
> - *                of the following flags (see string_escape_mem() for the
> - *                details):
> - *                  a - ESCAPE_ANY
> - *                  c - ESCAPE_SPECIAL
> - *                  h - ESCAPE_HEX
> - *                  n - ESCAPE_NULL
> - *                  o - ESCAPE_OCTAL
> - *                  p - ESCAPE_NP
> - *                  s - ESCAPE_SPACE
> - *                By default ESCAPE_ANY_NP is used.

Is there no way to retain these per-flag details in the kernel-doc?
Seems a shame to split up the docs if we can keep it together in here.

> - * - 'U' For a 16 byte UUID/GUID, it prints the UUID/GUID in the form
> - *       "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"
> - *       Options for %pU are:
> - *         b big endian lower case hex (default)
> - *         B big endian UPPER case hex
> - *         l little endian lower case hex
> - *         L little endian UPPER case hex
> - *           big endian output byte order is:
> - *             [0][1][2][3]-[4][5]-[6][7]-[8][9]-[10][11][12][13][14][15]
> - *           little endian output byte order is:
> - *             [3][2][1][0]-[5][4]-[7][6]-[8][9]-[10][11][12][13][14][15]
> - * - 'V' For a struct va_format which contains a format string * and va_list *,
> - *       call vsnprintf(->format, *->va_list).
> - *       Implements a "recursive vsnprintf".
> - *       Do not use this feature without some mechanism to verify the
> - *       correctness of the format string and va_list arguments.
> - * - 'K' For a kernel pointer that should be hidden from unprivileged users
> - * - 'NF' For a netdev_features_t
> - * - 'h[CDN]' For a variable-length buffer, it prints it as a hex string with
> - *            a certain separator (' ' by default):
> - *              C colon
> - *              D dash
> - *              N no separator
> - *            The maximum supported length is 64 bytes of the input. Consider
> - *            to use print_hex_dump() for the larger input.
> - * - 'a[pd]' For address types [p] phys_addr_t, [d] dma_addr_t and derivatives
> - *           (default assumed to be phys_addr_t, passed by reference)
> - * - 'd[234]' For a dentry name (optionally 2-4 last components)
> - * - 'D[234]' Same as 'd' but for a struct file
> - * - 'g' For block_device name (gendisk + partition number)
> - * - 'C' For a clock, it prints the name (Common Clock Framework) or address
> - *       (legacy clock framework) of the clock
> - * - 'Cn' For a clock, it prints the name (Common Clock Framework) or address
> - *        (legacy clock framework) of the clock
> - * - 'Cr' For a clock, it prints the current rate of the clock
> - * - 'G' For flags to be printed as a collection of symbolic strings that would
> - *       construct the specific value. Supported flags given by option:
> - *       p page flags (see struct page) given as pointer to unsigned long
> - *       g gfp flags (GFP_* and __GFP_*) given as pointer to gfp_t
> - *       v vma flags (VM_*) given as pointer to unsigned long
> - * - 'O' For a kobject based struct. Must be one of the following:
> - *       - 'OF[fnpPcCF]'  For a device tree object
> - *                        Without any optional arguments prints the full_name
> - *                        f device node full_name
> - *                        n device node name
> - *                        p device node phandle
> - *                        P device node path spec (name + @unit)
> - *                        F device node flags
> - *                        c major compatible string
> - *                        C full compatible string
> - *
> - * - 'x' For printing the address. Equivalent to "%lx".
> - *
> - * ** Please update also Documentation/printk-formats.txt when making changes **
> - *
>   * Note: The difference between 'S' and 'F' is that on ia64 and ppc64
>   * function pointers are really function descriptors, which contain a
>   * pointer to the real address.
> --
> 2.7.4
>

Nice work!

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH] doc: convert printk-formats.txt to rst

[Jonathan Corbet]

On Thu, 7 Dec 2017 14:50:42 -0800
Kees Cook <keescook@chromium.org> wrote:

> > +These guides contain general information useful when writing kernel code.
> > +
> > +.. toctree::
> > +   :maxdepth: 1
> > +
> > +   printk-formats  
> 
> I actually think this belongs in the kernel core API documentation
> list (Documentation/core-api/index.rst). I defer to Jon's opinion,
> though.

That's my opinion too, and what I had suggested in response to the 
previous version.  I suspect the pernicious influence of wombats! :)

jon

Re: [PATCH] doc: convert printk-formats.txt to rst

[Tobin C. Harding]

On Thu, Dec 07, 2017 at 02:50:42PM -0800, Kees Cook wrote:
> On Tue, Dec 5, 2017 at 5:45 PM, Tobin C. Harding <me@tobin.cc> wrote:
> > Documentation/printk-formats.txt is a candidate for conversion to
> > ReStructuredText format. Some effort has already been made to do this
> > conversion even thought the suffix is currently .txt
> >
> > Changes required to complete conversion
> >
> > - Add double backticks where needed.
> > - Add entry to Documentation/index.rst
> > - Use flat-table instead of ASCII table.
> > - Fix minor grammatical errors.
> > - Capitalize headers and correctly order heading adornments.
> > - Use 'Passed by reference' uniformly.
> > - Update pointer documentation around %px specifier.
> > - Fix erroneous double backticks (to commas).
> > - Simplify documentation for kobject.
> > - Convert lib/vsnprintf.c function docs to use kernel-docs and
> >   include in Documentation/printk-formats.rst
> 
> Awesome!

Hi Kees,

I've managed to muddle up the patches for this. New version went in as
part of a larger patch set. As you will no doubt see I've requested that
set to be dropped so I can implement your suggestions.

> >
> > Signed-off-by: Tobin C. Harding <me@tobin.cc>
> > ---
> >
> > The last two need special reviewing please. In particular the conversion
> > of kernel-docs in vsnprintf.c attempt was made to reduce documentation
> > duplication with comments in the source code being simplified in order
> > to be suitable for inclusion in Documentation/printk-formats.rst
> >
> > I used -M when formatting the patch. If you don't like the diff with
> > this flag just holla.
> >
> > thanks,
> > Tobin.
> >
> >  Documentation/index.rst                            |  10 +
> >  .../{printk-formats.txt => printk-formats.rst}     | 295 ++++++++++++---------
> >  lib/vsprintf.c                                     | 160 +++++------
> >  3 files changed, 235 insertions(+), 230 deletions(-)
> >  rename Documentation/{printk-formats.txt => printk-formats.rst} (61%)
> >
> > diff --git a/Documentation/index.rst b/Documentation/index.rst
> > index cb7f1ba5b3b1..83ace60efbe7 100644
> > --- a/Documentation/index.rst
> > +++ b/Documentation/index.rst
> > @@ -87,6 +87,16 @@ implementation.
> >
> >     sh/index
> >
> > +Miscellaneous documentation
> > +---------------------------
> > +
> > +These guides contain general information useful when writing kernel code.
> > +
> > +.. toctree::
> > +   :maxdepth: 1
> > +
> > +   printk-formats
> 
> I actually think this belongs in the kernel core API documentation
> list (Documentation/core-api/index.rst). I defer to Jon's opinion,
> though.

Your right, Jon has told me as such.

> > [...]
> > +.. kernel-doc:: lib/vsprintf.c
> > +     :doc: Extended Format Pointer Specifiers
> 
> Awesome!
> 
> > [...]
> > +For printing pointers when you *really* want to print the address. Please
> >  consider whether or not you are leaking sensitive information about the
> > -Kernel layout in memory before printing pointers with %px. %px is
> > -functionally equivalent to %lx. %px is preferred to %lx because it is more
> > -uniquely grep'able. If, in the future, we need to modify the way the Kernel
> > -handles printing pointers it will be nice to be able to find the call
> > -sites.
> > +kernel memory layout before printing pointers with ``%px``. ``%px`` is
> > +functionally equivalent to ``%lx`` (or ``%lu``). ``%px``, however, is
> > +preferable because it is more uniquely grep'able. If, in the future, we need
> > +to modify the way the Kernel handles printing pointers we will be better
> > +equipped to find the call sites.
> 
> nit? You de-capitalized Kernel at the start but not at the end. "the
> Kernel handles ..."

Yeah, this was intentional. I guess it's wrong. I'm struggling with
capitalization of 'kernel' at the moment. In previous discussion it was
suggested to me that when referring to _the_ Kernel it should be capital
because it implies the Linux Kernel. And when referring to what could be
any kernel it should be lower case. The sticking point, and why I did
this patch as it is, is that I never refer to any other kernels so its
hard to tell when it should be lowercase. On top of this, loads of other
developers just us lowercase all the time.

Grammar is harder than C :)

> > [...]
> > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> > index 01c3957b2de6..f9247b78e8ef 100644
> > --- a/lib/vsprintf.c
> > +++ b/lib/vsprintf.c
> > @@ -1727,115 +1727,73 @@ static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec)
> >         return number(buf, end, hashval, spec);
> >  }
> >
> > +/**
> > + * DOC: Extended Format Pointer Specifiers
> > + *
> > + * Briefly we handle the following extensions:
> > + *
> > + * - ``F`` - For symbolic function descriptor pointers with offset.
> > + * - ``f`` - For simple symbolic function names without offset.
> > + *
> > + * - ``S`` - For symbolic direct pointers with offset.
> > + * - ``s`` - For symbolic direct pointers without offset.
> > + * - ``[FfSs]R`` - As above with ``__builtin_extract_return_addr()`` translation.
> > + * - ``B`` - For backtraced symbolic direct pointers with offset.
> > + * - ``R`` - For decoded struct resource, e.g., [mem 0x0-0x1f 64bit pref].
> > + * - ``r`` - For raw struct resource, e.g., [mem 0x0-0x1f flags 0x201].
> > + * - ``b[l]`` - For a bitmap, the number of bits is determined by the field
> > + *   width which must be explicitly specified either as part of the format
> > + *   string ``32b[l]`` or through ``*b[l]``, ``[l]`` selects range-list format
> > + *   instead of hex format.
> > + * - ``M`` - For a 6-byte MAC address, it prints the address in the usual
> > + *   colon-separated hex notation.
> > + * - ``m`` - For a 6-byte MAC address, it prints the hex address without colons.
> > + * - ``MF`` - For a 6-byte MAC FDDI address, it prints the address with a
> > + *   dash-separated hex notation.
> > + * - ``[mM]R`` - For a 6-byte MAC address, Reverse order (Bluetooth).
> > + * - ``I[46]`` - For IPv4/IPv6 addresses printed in the usual way.
> > + * - ``I[S][pfs]`` - For generic IPv4/IPv6 address (struct sockaddr *) that falls
> > + *   back to ``[4]`` or ``[6]`` and is able to print port ``[p]``,
> > + *   flowinfo ``[f]``, scope ``[s]``.
> > + * - ``i[46]`` - For 'raw' IPv4/IPv6 addresses IPv6 omits the colons (01020304...0f)
> > + *   IPv4 uses dot-separated decimal with leading 0's (010.123.045.006).
> > + * - ``i[S][pfs]`` - For generic IPv4/IPv6 address (struct sockaddr *) that falls back
> > + *   to ``[4]`` or ``[6]`` (``[pfs]`` as above).
> > + * - ``[Ii][4S][hnbl]`` - For IPv4 addresses in host, network, big or little endian order.
> > + * - ``I[6S]c`` - For IPv6 addresses printed as per http://tools.ietf.org/html/rfc5952.
> > + * - ``E[achnops]`` - For an escaped buffer.
> > + * - ``U`` - For a 16 byte UUID/GUID.
> > + * - ``V`` - For a ``struct va_format`` which contains a format ``string *``
> > + *   and ``va_list *``.
> > + * - ``K`` -  For a kernel pointer that should be hidden from unprivileged users.
> > + * - ``NF`` - For a ``netdev_features_t``.
> > + * - ``h[CDN]`` - For a variable-length buffer.
> > + * - ``a[pd]`` - For address types ``[p] phys_addr_t``, ``[d] dma_addr_t`` and
> > + *   derivatives.
> > + * - ``d[234]`` - For a dentry name (optionally 2-4 last components).
> > + * - ``D[234]`` - Same as 'd' but for a struct file.
> > + * - ``g`` - For ``block_device`` name (gendisk + partition number).
> > + * - ``C[n]`` - For a clock, it prints the name (Common Clock Framework) or
> > + *   address (legacy clock framework) of the clock. ``[n]`` is optional.
> > + * - ``Cr`` - For a clock, it prints the current rate of the clock.
> > + * - ``G`` - For flags to be printed as a collection of symbolic strings that
> > + *   would construct the specific value.
> > + * - ``O`` - For a kobject based struct (device node).
> > + * - ``x`` - For printing the address. Equivalent to ``%lx``.
> > + */
> > +
> >  /*
> >   * Show a '%p' thing.  A kernel extension is that the '%p' is followed
> >   * by an extra set of alphanumeric characters that are extended format
> >   * specifiers.
> >   *
> > + * Please see Documentation/printk-formats.rst for fuller description
> > + * of specifier extensions. Also please update this file when making
> > + * changes.
> 
> Perhaps "... please update the kernel-doc above when making changes to
> this file." ?

this got changed

	's/this/that'

based on previous review.

> > + *
> >   * Please update scripts/checkpatch.pl when adding/removing conversion
> >   * characters.  (Search for "check for vsprintf extension").
> >   *
> > - * Right now we handle:
> > - *
> > - * - 'F' For symbolic function descriptor pointers with offset
> > - * - 'f' For simple symbolic function names without offset
> > - * - 'S' For symbolic direct pointers with offset
> > - * - 's' For symbolic direct pointers without offset
> > - * - '[FfSs]R' as above with __builtin_extract_return_addr() translation
> > - * - 'B' For backtraced symbolic direct pointers with offset
> > - * - 'R' For decoded struct resource, e.g., [mem 0x0-0x1f 64bit pref]
> > - * - 'r' For raw struct resource, e.g., [mem 0x0-0x1f flags 0x201]
> > - * - 'b[l]' For a bitmap, the number of bits is determined by the field
> > - *       width which must be explicitly specified either as part of the
> > - *       format string '%32b[l]' or through '%*b[l]', [l] selects
> > - *       range-list format instead of hex format
> > - * - 'M' For a 6-byte MAC address, it prints the address in the
> > - *       usual colon-separated hex notation
> > - * - 'm' For a 6-byte MAC address, it prints the hex address without colons
> > - * - 'MF' For a 6-byte MAC FDDI address, it prints the address
> > - *       with a dash-separated hex notation
> > - * - '[mM]R' For a 6-byte MAC address, Reverse order (Bluetooth)
> > - * - 'I' [46] for IPv4/IPv6 addresses printed in the usual way
> > - *       IPv4 uses dot-separated decimal without leading 0's (1.2.3.4)
> > - *       IPv6 uses colon separated network-order 16 bit hex with leading 0's
> > - *       [S][pfs]
> > - *       Generic IPv4/IPv6 address (struct sockaddr *) that falls back to
> > - *       [4] or [6] and is able to print port [p], flowinfo [f], scope [s]
> > - * - 'i' [46] for 'raw' IPv4/IPv6 addresses
> > - *       IPv6 omits the colons (01020304...0f)
> > - *       IPv4 uses dot-separated decimal with leading 0's (010.123.045.006)
> > - *       [S][pfs]
> > - *       Generic IPv4/IPv6 address (struct sockaddr *) that falls back to
> > - *       [4] or [6] and is able to print port [p], flowinfo [f], scope [s]
> > - * - '[Ii][4S][hnbl]' IPv4 addresses in host, network, big or little endian order
> > - * - 'I[6S]c' for IPv6 addresses printed as specified by
> > - *       http://tools.ietf.org/html/rfc5952
> > - * - 'E[achnops]' For an escaped buffer, where rules are defined by combination
> > - *                of the following flags (see string_escape_mem() for the
> > - *                details):
> > - *                  a - ESCAPE_ANY
> > - *                  c - ESCAPE_SPECIAL
> > - *                  h - ESCAPE_HEX
> > - *                  n - ESCAPE_NULL
> > - *                  o - ESCAPE_OCTAL
> > - *                  p - ESCAPE_NP
> > - *                  s - ESCAPE_SPACE
> > - *                By default ESCAPE_ANY_NP is used.
> 
> Is there no way to retain these per-flag details in the kernel-doc?
> Seems a shame to split up the docs if we can keep it together in here.

Good point, I'll put these back in.

> > - * - 'U' For a 16 byte UUID/GUID, it prints the UUID/GUID in the form
> > - *       "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"
> > - *       Options for %pU are:
> > - *         b big endian lower case hex (default)
> > - *         B big endian UPPER case hex
> > - *         l little endian lower case hex
> > - *         L little endian UPPER case hex
> > - *           big endian output byte order is:
> > - *             [0][1][2][3]-[4][5]-[6][7]-[8][9]-[10][11][12][13][14][15]
> > - *           little endian output byte order is:
> > - *             [3][2][1][0]-[5][4]-[7][6]-[8][9]-[10][11][12][13][14][15]
> > - * - 'V' For a struct va_format which contains a format string * and va_list *,
> > - *       call vsnprintf(->format, *->va_list).
> > - *       Implements a "recursive vsnprintf".
> > - *       Do not use this feature without some mechanism to verify the
> > - *       correctness of the format string and va_list arguments.
> > - * - 'K' For a kernel pointer that should be hidden from unprivileged users
> > - * - 'NF' For a netdev_features_t
> > - * - 'h[CDN]' For a variable-length buffer, it prints it as a hex string with
> > - *            a certain separator (' ' by default):
> > - *              C colon
> > - *              D dash
> > - *              N no separator
> > - *            The maximum supported length is 64 bytes of the input. Consider
> > - *            to use print_hex_dump() for the larger input.
> > - * - 'a[pd]' For address types [p] phys_addr_t, [d] dma_addr_t and derivatives
> > - *           (default assumed to be phys_addr_t, passed by reference)
> > - * - 'd[234]' For a dentry name (optionally 2-4 last components)
> > - * - 'D[234]' Same as 'd' but for a struct file
> > - * - 'g' For block_device name (gendisk + partition number)
> > - * - 'C' For a clock, it prints the name (Common Clock Framework) or address
> > - *       (legacy clock framework) of the clock
> > - * - 'Cn' For a clock, it prints the name (Common Clock Framework) or address
> > - *        (legacy clock framework) of the clock
> > - * - 'Cr' For a clock, it prints the current rate of the clock
> > - * - 'G' For flags to be printed as a collection of symbolic strings that would
> > - *       construct the specific value. Supported flags given by option:
> > - *       p page flags (see struct page) given as pointer to unsigned long
> > - *       g gfp flags (GFP_* and __GFP_*) given as pointer to gfp_t
> > - *       v vma flags (VM_*) given as pointer to unsigned long
> > - * - 'O' For a kobject based struct. Must be one of the following:
> > - *       - 'OF[fnpPcCF]'  For a device tree object
> > - *                        Without any optional arguments prints the full_name
> > - *                        f device node full_name
> > - *                        n device node name
> > - *                        p device node phandle
> > - *                        P device node path spec (name + @unit)
> > - *                        F device node flags
> > - *                        c major compatible string
> > - *                        C full compatible string
> > - *
> > - * - 'x' For printing the address. Equivalent to "%lx".
> > - *
> > - * ** Please update also Documentation/printk-formats.txt when making changes **
> > - *
> >   * Note: The difference between 'S' and 'F' is that on ia64 and ppc64
> >   * function pointers are really function descriptors, which contain a
> >   * pointer to the real address.
> > --
> > 2.7.4
> >
> 
> Nice work!

Cheers Kees. FTR, changes to implement are:

 - Fix the capitalization of 'kernel'.
 - Add ESCAPE_* flags back into kernel-docs in lib/vsprintf.c

thanks,
Tobin.

Re: [PATCH] doc: convert printk-formats.txt to rst

[Tobin C. Harding]

On Thu, Dec 07, 2017 at 04:01:46PM -0700, Jonathan Corbet wrote:
> On Thu, 7 Dec 2017 14:50:42 -0800
> Kees Cook <keescook@chromium.org> wrote:
> 
> > > +These guides contain general information useful when writing kernel code.
> > > +
> > > +.. toctree::
> > > +   :maxdepth: 1
> > > +
> > > +   printk-formats  
> > 
> > I actually think this belongs in the kernel core API documentation
> > list (Documentation/core-api/index.rst). I defer to Jon's opinion,
> > though.
> 
> That's my opinion too, and what I had suggested in response to the 
> previous version.  I suspect the pernicious influence of wombats! :)

Steady on! Don't overuse it, you'll hurt my feelings :) Just kidding. I
believe I got this right in the current version. If you are getting
confused trying to follow these patches just try and follow the
discussion my previous effort when hashing %p

I'll let it settle a bit and re-spin after the weekend.

thanks,
Tobin.

Re: [PATCH] doc: convert printk-formats.txt to rst

[Kees Cook]

On Thu, Dec 7, 2017 at 3:44 PM, Tobin C. Harding <me@tobin.cc> wrote:
> Cheers Kees. FTR, changes to implement are:
>
>  - Fix the capitalization of 'kernel'.

I don't really have an opinion about which way is right. I personally
don't capitalize it unless I speak about it as a single thing "The
Linux Kernel". In this case I just noticed you had mixed usage.

>  - Add ESCAPE_* flags back into kernel-docs in lib/vsprintf.c

I actually meant each of the sections. Several of the formats have
per-item breakdowns that went missing in the new kernel-doc (ESCAPE_*
was just an example).

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH] doc: convert printk-formats.txt to rst

[Tobin C. Harding]

On Thu, Dec 07, 2017 at 04:19:56PM -0800, Kees Cook wrote:
> On Thu, Dec 7, 2017 at 3:44 PM, Tobin C. Harding <me@tobin.cc> wrote:
> > Cheers Kees. FTR, changes to implement are:
> >
> >  - Fix the capitalization of 'kernel'.
> 
> I don't really have an opinion about which way is right. I personally
> don't capitalize it unless I speak about it as a single thing "The
> Linux Kernel". In this case I just noticed you had mixed usage.
> 
> >  - Add ESCAPE_* flags back into kernel-docs in lib/vsprintf.c
> 
> I actually meant each of the sections. Several of the formats have
> per-item breakdowns that went missing in the new kernel-doc (ESCAPE_*
> was just an example).

Oh dear, you don't like that. This is actually the part of the patch
that I was least sure about doing. I'm happy to revert, can I give you
my thought process for comment?

When the kernel-docs get included into printk-formats.rst it seems
overly verbose to have all the information given twice. And then it
seems odd to bother having the extra descriptions in printk-formats.rst
if _all_ the required information is already in the kernel-docs?

So I guessed that it would be nice for devs to get a bit of a hint at
the specifiers when having lib/vsprintf.c open (and they have the code
too) then if they needed more information going to printk-formats.rst.

Also, since there is more space in printk-fomats.rst the info can be
spaced better and easier to read.

Your thoughts?

thanks,
Tobin.

Re: [PATCH] doc: convert printk-formats.txt to rst

[Kees Cook]

On Thu, Dec 7, 2017 at 4:46 PM, Tobin C. Harding <me@tobin.cc> wrote:
> On Thu, Dec 07, 2017 at 04:19:56PM -0800, Kees Cook wrote:
>> On Thu, Dec 7, 2017 at 3:44 PM, Tobin C. Harding <me@tobin.cc> wrote:
>> > Cheers Kees. FTR, changes to implement are:
>> >
>> >  - Fix the capitalization of 'kernel'.
>>
>> I don't really have an opinion about which way is right. I personally
>> don't capitalize it unless I speak about it as a single thing "The
>> Linux Kernel". In this case I just noticed you had mixed usage.
>>
>> >  - Add ESCAPE_* flags back into kernel-docs in lib/vsprintf.c
>>
>> I actually meant each of the sections. Several of the formats have
>> per-item breakdowns that went missing in the new kernel-doc (ESCAPE_*
>> was just an example).
>
> Oh dear, you don't like that. This is actually the part of the patch
> that I was least sure about doing. I'm happy to revert, can I give you
> my thought process for comment?
>
> When the kernel-docs get included into printk-formats.rst it seems
> overly verbose to have all the information given twice. And then it
> seems odd to bother having the extra descriptions in printk-formats.rst
> if _all_ the required information is already in the kernel-docs?
>
> So I guessed that it would be nice for devs to get a bit of a hint at
> the specifiers when having lib/vsprintf.c open (and they have the code
> too) then if they needed more information going to printk-formats.rst.
>
> Also, since there is more space in printk-fomats.rst the info can be
> spaced better and easier to read.
>
> Your thoughts?

Well ... my sense is that lib/vsprintf.c should remain the canonical
documentation. Anyone working on the code has the docs all together in
one file. If it helps the .rst file to reformat the comments into
kernel-doc, that's fine, but it shouldn't reduce the detail that is
present, IMO. Now, expanding on it in printk-formats.rst is certainly
a great idea, but I don't think it should come at the expense of
someone just reading through vsprintf.c. That said, I can certainly
see that redundancy is annoying, and it's possible for
printk-formats.rst and vsprintf.c get get out of sync, but that
doesn't seem to be a new problem.

I'd be curious to see what Jon or Joe think about this.

(Perhaps the best first step would be to leave vsprintf.c as-is
without kernel-doc-ification?)

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH] doc: convert printk-formats.txt to rst

[Joe Perches]

On Fri, 2017-12-08 at 13:06 -0800, Kees Cook wrote:
> Well ... my sense is that lib/vsprintf.c should remain the canonical
> documentation.

I agree.

> Anyone working on the code has the docs all together in
> one file. If it helps the .rst file to reformat the comments into
> kernel-doc, that's fine, but it shouldn't reduce the detail that is
> present, IMO. Now, expanding on it in printk-formats.rst is certainly
> a great idea, but I don't think it should come at the expense of
> someone just reading through vsprintf.c. That said, I can certainly
> see that redundancy is annoying, and it's possible for
> printk-formats.rst and vsprintf.c get get out of sync, but that
> doesn't seem to be a new problem.

Nor has it been a real problem in practice.

There is a comment in vsprintf.c that tells people
to update the doc.

 * ** Please update also Documentation/printk-formats.txt when making changes **
> 
> I'd be curious to see what Jon or Joe think about this.
> 
> (Perhaps the best first step would be to leave vsprintf.c as-is
> without kernel-doc-ification?)

I think adding kernel-doc to vsprintf.c is unnecessary.

Outside of the documentation, what could be useful is for
someone to add a tool to verify %p<foo> extension to
the typeof address actually passed as an argument.

Re: [PATCH] doc: convert printk-formats.txt to rst

[Tobin C. Harding]

On Fri, Dec 08, 2017 at 01:22:37PM -0800, Joe Perches wrote:
> On Fri, 2017-12-08 at 13:06 -0800, Kees Cook wrote:
> > Well ... my sense is that lib/vsprintf.c should remain the canonical
> > documentation.
> 
> I agree.
> 
> > Anyone working on the code has the docs all together in
> > one file. If it helps the .rst file to reformat the comments into
> > kernel-doc, that's fine, but it shouldn't reduce the detail that is
> > present, IMO. Now, expanding on it in printk-formats.rst is certainly
> > a great idea, but I don't think it should come at the expense of
> > someone just reading through vsprintf.c. That said, I can certainly
> > see that redundancy is annoying, and it's possible for
> > printk-formats.rst and vsprintf.c get get out of sync, but that
> > doesn't seem to be a new problem.
> 
> Nor has it been a real problem in practice.
> 
> There is a comment in vsprintf.c that tells people
> to update the doc.
> 
>  * ** Please update also Documentation/printk-formats.txt when making changes **
> > 
> > I'd be curious to see what Jon or Joe think about this.
> > 
> > (Perhaps the best first step would be to leave vsprintf.c as-is
> > without kernel-doc-ification?)
> 
> I think adding kernel-doc to vsprintf.c is unnecessary.

Ok, thanks. Will re-spin without kernel-doc-ification in vsprintf.c

> Outside of the documentation, what could be useful is for
> someone to add a tool to verify %p<foo> extension to
> the typeof address actually passed as an argument.

This sounds interesting to work no. At first glance I have no idea how
one would go about this. Some form of static analysis would be a good
place to start, right? I'd like to allocate some cycles to this, any
pointers most appreciated.

thanks,
Tobin.

Re: [PATCH] doc: convert printk-formats.txt to rst

[Joe Perches]

On Sat, 2017-12-09 at 12:27 +1100, Tobin C. Harding wrote:
> On Fri, Dec 08, 2017 at 01:22:37PM -0800, Joe Perches wrote:

> > Outside of the documentation, what could be useful is for
> > someone to add a tool to verify %p<foo> extension to
> > the typeof address actually passed as an argument.
> 
> This sounds interesting to work no. At first glance I have no idea how
> one would go about this. Some form of static analysis would be a good
> place to start, right? I'd like to allocate some cycles to this, any
> pointers most appreciated.

A gcc-plugin would likely work best.

There was some discussion about such a thing here:
http://www.openwall.com/lists/kernel-hardening/2017/02/14/38

I vaguely recall someone else doing a broader use tool
which I believe was not smatch, but my google-fu isn't
finding it.

It might have been coccinelle based.

Re: [PATCH] doc: convert printk-formats.txt to rst

[Tobin C. Harding]

[Adding Laura]

On Fri, Dec 08, 2017 at 06:18:45PM -0800, Joe Perches wrote:
> On Sat, 2017-12-09 at 12:27 +1100, Tobin C. Harding wrote:
> > On Fri, Dec 08, 2017 at 01:22:37PM -0800, Joe Perches wrote:
> 
> > > Outside of the documentation, what could be useful is for
> > > someone to add a tool to verify %p<foo> extension to
> > > the typeof address actually passed as an argument.
> > 
> > This sounds interesting to work no. At first glance I have no idea how
> > one would go about this. Some form of static analysis would be a good
> > place to start, right? I'd like to allocate some cycles to this, any
> > pointers most appreciated.
> 
> A gcc-plugin would likely work best.

What's the learning curve like in your opinion to do a gcc-plugin. I
recall reading someplace 'deep understanding of how the compiler works'
or some such thing. I suppose reading the Dragon book would be a good
place to start?

We could also catch pointers being cast to longs and printed with %x
(and %u) or so I would guess.

> There was some discussion about such a thing here:
> http://www.openwall.com/lists/kernel-hardening/2017/02/14/38

Did you make much progress with this Laura?

> I vaguely recall someone else doing a broader use tool
> which I believe was not smatch, but my google-fu isn't
> finding it.
> 
> It might have been coccinelle based.

thanks,
Tobin.

Re: [PATCH] doc: convert printk-formats.txt to rst

[Dan Carpenter]

On Fri, Dec 08, 2017 at 06:18:45PM -0800, Joe Perches wrote:
> On Sat, 2017-12-09 at 12:27 +1100, Tobin C. Harding wrote:
> > On Fri, Dec 08, 2017 at 01:22:37PM -0800, Joe Perches wrote:
> 
> > > Outside of the documentation, what could be useful is for
> > > someone to add a tool to verify %p<foo> extension to
> > > the typeof address actually passed as an argument.
> > 
> > This sounds interesting to work no. At first glance I have no idea how
> > one would go about this. Some form of static analysis would be a good
> > place to start, right? I'd like to allocate some cycles to this, any
> > pointers most appreciated.
> 
> A gcc-plugin would likely work best.
> 
> There was some discussion about such a thing here:
> http://www.openwall.com/lists/kernel-hardening/2017/02/14/38
> 
> I vaguely recall someone else doing a broader use tool
> which I believe was not smatch, but my google-fu isn't
> finding it.

Yeah.  Smatch has a check for this.  Rasmus Villemoes wrote it.

regards,
dan carpenter

Re: [PATCH] doc: convert printk-formats.txt to rst

[Kees Cook]

On Sat, Dec 9, 2017 at 3:48 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> On Fri, Dec 08, 2017 at 06:18:45PM -0800, Joe Perches wrote:
>> On Sat, 2017-12-09 at 12:27 +1100, Tobin C. Harding wrote:
>> > On Fri, Dec 08, 2017 at 01:22:37PM -0800, Joe Perches wrote:
>>
>> > > Outside of the documentation, what could be useful is for
>> > > someone to add a tool to verify %p<foo> extension to
>> > > the typeof address actually passed as an argument.
>> >
>> > This sounds interesting to work no. At first glance I have no idea how
>> > one would go about this. Some form of static analysis would be a good
>> > place to start, right? I'd like to allocate some cycles to this, any
>> > pointers most appreciated.
>>
>> A gcc-plugin would likely work best.
>>
>> There was some discussion about such a thing here:
>> http://www.openwall.com/lists/kernel-hardening/2017/02/14/38
>>
>> I vaguely recall someone else doing a broader use tool
>> which I believe was not smatch, but my google-fu isn't
>> finding it.
>
> Yeah.  Smatch has a check for this.  Rasmus Villemoes wrote it.

There's been some other work on format strings by Rasmus too. Thread
is here, which I still haven't caught back up on:

http://www.openwall.com/lists/kernel-hardening/2017/11/08/25

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH] doc: convert printk-formats.txt to rst

[Laura Abbott]

On 12/08/2017 10:33 PM, Tobin C. Harding wrote:
> [Adding Laura]
> 
> On Fri, Dec 08, 2017 at 06:18:45PM -0800, Joe Perches wrote:
>> On Sat, 2017-12-09 at 12:27 +1100, Tobin C. Harding wrote:
>>> On Fri, Dec 08, 2017 at 01:22:37PM -0800, Joe Perches wrote:
>>
>>>> Outside of the documentation, what could be useful is for
>>>> someone to add a tool to verify %p<foo> extension to
>>>> the typeof address actually passed as an argument.
>>>
>>> This sounds interesting to work no. At first glance I have no idea how
>>> one would go about this. Some form of static analysis would be a good
>>> place to start, right? I'd like to allocate some cycles to this, any
>>> pointers most appreciated.
>>
>> A gcc-plugin would likely work best.
> 
> What's the learning curve like in your opinion to do a gcc-plugin. I
> recall reading someplace 'deep understanding of how the compiler works'
> or some such thing. I suppose reading the Dragon book would be a good
> place to start?
> 

The hardest part of doing a gcc-plugin is understanding the gccisms.
There isn't much documentation and most of what there is ends up
being "here's how you hook into the compiler at point X" without
showing how you do anything with it. The Dragon book (also known
as "Compilers: Principles, Techniques, and Tools" for those who
haven't heard of it before) is a great resource for general compiler
concepts but it's not helpful for gcc-specific work.

Writing about some of my experiments on my gcc-plugins has been
on my TODO list for a while. 2018 resolution on actually finishing
it perhaps.

> We could also catch pointers being cast to longs and printed with %x
> (and %u) or so I would guess.
> 
>> There was some discussion about such a thing here:
>> http://www.openwall.com/lists/kernel-hardening/2017/02/14/38
> 
> Did you make much progress with this Laura?
> 

Not particularly. I wanted to re-use the kernel's print functionality
in the plugin to automatically check new formats but I went
down a rathole trying to make it work and got side tracked with
other things and never picked it up again.

>> I vaguely recall someone else doing a broader use tool
>> which I believe was not smatch, but my google-fu isn't
>> finding it.
>>
>> It might have been coccinelle based.
> 
> thanks,
> Tobin.
> 

Thanks,
Laura