All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next v2] bpf: relax constraints on formatting for eBPF helper documentation
@ 2018-05-02 13:20 Quentin Monnet
  2018-05-02 15:48 ` Daniel Borkmann
  0 siblings, 1 reply; 2+ messages in thread
From: Quentin Monnet @ 2018-05-02 13:20 UTC (permalink / raw)
  To: daniel, ast; +Cc: dsahern, yhs, ecree, netdev, oss-drivers, quentin.monnet

The Python script used to parse and extract eBPF helpers documentation
from include/uapi/linux/bpf.h expects a very specific formatting for the
descriptions (single dot represents a space, '>' stands for a tab):

    /*
     ...
     *.int bpf_helper(list of arguments)
     *.>    Description
     *.>    >       Start of description
     *.>    >       Another line of description
     *.>    >       And yet another line of description
     *.>    Return
     *.>    >       0 on success, or a negative error in case of failure
     ...
     */

This is too strict, and painful for developers who wants to add
documentation for new helpers. Worse, it is extremely difficult to check
that the formatting is correct during reviews. Change the format
expected by the script and make it more flexible. The script now works
whether or not the initial space (right after the star) is present, and
accepts both tabs and white spaces (or a combination of both) for
indenting description sections and contents.

Concretely, something like the following would now be supported:

    /*
     ...
     *int bpf_helper(list of arguments)
     *......Description
     *.>    >       Start of description...
     *>     >       Another line of description
     *..............And yet another line of description
     *>     Return
     *.>    ........0 on success, or a negative error in case of failure
     ...
     */

While at it, remove unnecessary carets from each regex used with match()
in the script. They are redundant, as match() tries to match from the
beginning of the string by default.

v2: Remove unnecessary caret when a regex is used with match().

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 scripts/bpf_helpers_doc.py | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/scripts/bpf_helpers_doc.py b/scripts/bpf_helpers_doc.py
index 30ba0fee36e4..8f59897fbda1 100755
--- a/scripts/bpf_helpers_doc.py
+++ b/scripts/bpf_helpers_doc.py
@@ -39,9 +39,9 @@ class Helper(object):
         Break down helper function protocol into smaller chunks: return type,
         name, distincts arguments.
         """
-        arg_re = re.compile('^((const )?(struct )?(\w+|...))( (\**)(\w+))?$')
+        arg_re = re.compile('((const )?(struct )?(\w+|...))( (\**)(\w+))?$')
         res = {}
-        proto_re = re.compile('^(.+) (\**)(\w+)\(((([^,]+)(, )?){1,5})\)$')
+        proto_re = re.compile('(.+) (\**)(\w+)\(((([^,]+)(, )?){1,5})\)$')
 
         capture = proto_re.match(self.proto)
         res['ret_type'] = capture.group(1)
@@ -87,7 +87,7 @@ class HeaderParser(object):
         #   - Same as above, with "const" and/or "struct" in front of type
         #   - "..." (undefined number of arguments, for bpf_trace_printk())
         # There is at least one term ("void"), and at most five arguments.
-        p = re.compile('^ \* ((.+) \**\w+\((((const )?(struct )?(\w+|\.\.\.)( \**\w+)?)(, )?){1,5}\))$')
+        p = re.compile(' \* ?((.+) \**\w+\((((const )?(struct )?(\w+|\.\.\.)( \**\w+)?)(, )?){1,5}\))$')
         capture = p.match(self.line)
         if not capture:
             raise NoHelperFound
@@ -95,7 +95,7 @@ class HeaderParser(object):
         return capture.group(1)
 
     def parse_desc(self):
-        p = re.compile('^ \* \tDescription$')
+        p = re.compile(' \* ?(?:\t| {6,8})Description$')
         capture = p.match(self.line)
         if not capture:
             # Helper can have empty description and we might be parsing another
@@ -109,7 +109,7 @@ class HeaderParser(object):
             if self.line == ' *\n':
                 desc += '\n'
             else:
-                p = re.compile('^ \* \t\t(.*)')
+                p = re.compile(' \* ?(?:\t| {6,8})(?:\t| {8})(.*)')
                 capture = p.match(self.line)
                 if capture:
                     desc += capture.group(1) + '\n'
@@ -118,7 +118,7 @@ class HeaderParser(object):
         return desc
 
     def parse_ret(self):
-        p = re.compile('^ \* \tReturn$')
+        p = re.compile(' \* ?(?:\t| {6,8})Return$')
         capture = p.match(self.line)
         if not capture:
             # Helper can have empty retval and we might be parsing another
@@ -132,7 +132,7 @@ class HeaderParser(object):
             if self.line == ' *\n':
                 ret += '\n'
             else:
-                p = re.compile('^ \* \t\t(.*)')
+                p = re.compile(' \* ?(?:\t| {6,8})(?:\t| {8})(.*)')
                 capture = p.match(self.line)
                 if capture:
                     ret += capture.group(1) + '\n'
-- 
2.14.1

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

* Re: [PATCH bpf-next v2] bpf: relax constraints on formatting for eBPF helper documentation
  2018-05-02 13:20 [PATCH bpf-next v2] bpf: relax constraints on formatting for eBPF helper documentation Quentin Monnet
@ 2018-05-02 15:48 ` Daniel Borkmann
  0 siblings, 0 replies; 2+ messages in thread
From: Daniel Borkmann @ 2018-05-02 15:48 UTC (permalink / raw)
  To: Quentin Monnet, ast; +Cc: dsahern, yhs, ecree, netdev, oss-drivers

On 05/02/2018 03:20 PM, Quentin Monnet wrote:
> The Python script used to parse and extract eBPF helpers documentation
> from include/uapi/linux/bpf.h expects a very specific formatting for the
> descriptions (single dot represents a space, '>' stands for a tab):
> 
>     /*
>      ...
>      *.int bpf_helper(list of arguments)
>      *.>    Description
>      *.>    >       Start of description
>      *.>    >       Another line of description
>      *.>    >       And yet another line of description
>      *.>    Return
>      *.>    >       0 on success, or a negative error in case of failure
>      ...
>      */
> 
> This is too strict, and painful for developers who wants to add
> documentation for new helpers. Worse, it is extremely difficult to check
> that the formatting is correct during reviews. Change the format
> expected by the script and make it more flexible. The script now works
> whether or not the initial space (right after the star) is present, and
> accepts both tabs and white spaces (or a combination of both) for
> indenting description sections and contents.
> 
> Concretely, something like the following would now be supported:
> 
>     /*
>      ...
>      *int bpf_helper(list of arguments)
>      *......Description
>      *.>    >       Start of description...
>      *>     >       Another line of description
>      *..............And yet another line of description
>      *>     Return
>      *.>    ........0 on success, or a negative error in case of failure
>      ...
>      */
> 
> While at it, remove unnecessary carets from each regex used with match()
> in the script. They are redundant, as match() tries to match from the
> beginning of the string by default.
> 
> v2: Remove unnecessary caret when a regex is used with match().
> 
> Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>

Applied to bpf-next, thanks Quentin!

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

end of thread, other threads:[~2018-05-02 15:48 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-02 13:20 [PATCH bpf-next v2] bpf: relax constraints on formatting for eBPF helper documentation Quentin Monnet
2018-05-02 15:48 ` Daniel Borkmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.