netdev - Re: [PATCH bpf-next] bpf: relax constraints on formatting for eBPF helper documentation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ecc5415e-0a60-ba6b-84a7-d39bb8b4a0d5@solarflare.com>
Date:   Mon, 30 Apr 2018 17:33:41 +0100
From:   Edward Cree <ecree@...arflare.com>
To:     Quentin Monnet <quentin.monnet@...ronome.com>,
        <daniel@...earbox.net>, <ast@...nel.org>
CC:     <dsahern@...il.com>, <yhs@...com>, <netdev@...r.kernel.org>,
        <oss-drivers@...ronome.com>
Subject: Re: [PATCH bpf-next] bpf: relax constraints on formatting for eBPF
 helper documentation

On 30/04/18 16:59, 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 dots represent 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 extremelly 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
>      ...
>      */
>
> Signed-off-by: Quentin Monnet <quentin.monnet@...ronome.com>
> ---
>  scripts/bpf_helpers_doc.py | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/scripts/bpf_helpers_doc.py b/scripts/bpf_helpers_doc.py
> index 30ba0fee36e4..717547e6f0a6 100755
> --- a/scripts/bpf_helpers_doc.py
> +++ b/scripts/bpf_helpers_doc.py
> @@ -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}\))$')
The proper coding style for such things is to go straight to tabs after
 the star and not have the space.  So if we're going to make the script
 flexible here (and leave coding style enforcement to other tools such
 as checkpatch), maybe the regexen should just begin '^ \*\s+' and avoid
 relying on counting indentation to delimit sections (e.g. scan for the
 section headers like '^ \*\s+Description$' instead).
Btw, leading '^' is unnecessary as re.match() is already implicitly
 anchored at start-of-string.  (The trailing '$' are still needed.)

-Ed