diff mbox series

[bpf-next] libbpf: Add API documentation convention guidelines

Message ID 20211004215644.497327-1-grantseltzer@gmail.com (mailing list archive)
State Accepted
Delegated to: BPF
Headers show
Series [bpf-next] libbpf: Add API documentation convention guidelines | expand

Checks

Context Check Description
netdev/cover_letter success Single patches do not need cover letters
netdev/fixes_present success Fixes tag not required for -next series
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for bpf-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 11 maintainers not CCed: netdev@vger.kernel.org kafai@fb.com daniel@iogearbox.net yhs@fb.com john.fastabend@gmail.com rdunlap@infradead.org ast@kernel.org songliubraving@fb.com corbet@lwn.net kpsingh@kernel.org linux-doc@vger.kernel.org
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success No Fixes tag
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 46 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success No static functions without inline keyword in header files
bpf/vmtest-bpf-next success VM_Test
bpf/vmtest-bpf-next-PR fail merge-conflict

Commit Message

Grant Seltzer Richman Oct. 4, 2021, 9:56 p.m. UTC 
From: Grant Seltzer <grantseltzer@gmail.com>

This adds a section to the documentation for libbpf
naming convention which describes how to document
API features in libbpf, specifically the format of
which API doc comments need to conform to.

Signed-off-by: Grant Seltzer <grantseltzer@gmail.com>
---
 .../bpf/libbpf/libbpf_naming_convention.rst   | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)

Comments

Song Liu Oct. 5, 2021, 1 a.m. UTC | #1 
On Mon, Oct 4, 2021 at 4:51 PM grantseltzer <grantseltzer@gmail.com> wrote:
>
> From: Grant Seltzer <grantseltzer@gmail.com>
>
> This adds a section to the documentation for libbpf
> naming convention which describes how to document
> API features in libbpf, specifically the format of
> which API doc comments need to conform to.
>
> Signed-off-by: Grant Seltzer <grantseltzer@gmail.com>

Acked-by: Song Liu <songliubraving@fb.com>

> ---
>  .../bpf/libbpf/libbpf_naming_convention.rst   | 40 +++++++++++++++++++
>  1 file changed, 40 insertions(+)
>
> diff --git a/Documentation/bpf/libbpf/libbpf_naming_convention.rst b/Documentation/bpf/libbpf/libbpf_naming_convention.rst
> index 9c68d5014ff1..5f42f172987a 100644
> --- a/Documentation/bpf/libbpf/libbpf_naming_convention.rst
> +++ b/Documentation/bpf/libbpf/libbpf_naming_convention.rst
> @@ -150,6 +150,46 @@ mirror of the mainline's version of libbpf for a stand-alone build.
>  However, all changes to libbpf's code base must be upstreamed through
>  the mainline kernel tree.
>
> +
> +API documentation convention
> +============================
> +
> +The libbpf API is documented via comments above definitions in
> +header files. These comments can be rendered by doxygen and sphinx
> +for well organized html output. This section describes the
> +convention in which these comments should be formated.
> +
> +Here is an example from btf.h:
> +
> +.. code-block:: c
> +
> +        /**
> +        * @brief **btf__new()** creates a new instance of a BTF object from the raw
> +        * bytes of an ELF's BTF section
> +        * @param data raw bytes
> +        * @param size number of bytes passed in `data`
> +        * @return new BTF object instance which has to be eventually freed with
> +        * **btf__free()**
> +        *
> +        * On error, error-code-encoded-as-pointer is returned, not a NULL. To extract
> +        * error code from such a pointer `libbpf_get_error()` should be used. If
> +        * `libbpf_set_strict_mode(LIBBPF_STRICT_CLEAN_PTRS)` is enabled, NULL is
> +        * returned on error instead. In both cases thread-local `errno` variable is
> +        * always set to error code as well.
> +        */
> +
> +The comment must start with a block comment of the form '/**'.
> +
> +The documentation always starts with a @brief directive. This line is a short
> +description about this API. It starts with the name of the API, denoted in bold
> +like so: **api_name**. Please include an open and close parenthesis if this is a
> +function. Follow with the short description of the API. A longer form description
> +can be added below the last directive, at the bottom of the comment.
> +
> +Parameters are denoted with the @param directive, there should be one for each
> +parameter. If this is a function with a non-void return, use the @return directive
> +to document it.
> +
>  License
>  -------------------
>
> --
> 2.31.1
>
Andrii Nakryiko Oct. 6, 2021, 5:29 p.m. UTC | #2 
On Mon, Oct 4, 2021 at 6:00 PM Song Liu <song@kernel.org> wrote:
>
> On Mon, Oct 4, 2021 at 4:51 PM grantseltzer <grantseltzer@gmail.com> wrote:
> >
> > From: Grant Seltzer <grantseltzer@gmail.com>
> >
> > This adds a section to the documentation for libbpf
> > naming convention which describes how to document
> > API features in libbpf, specifically the format of
> > which API doc comments need to conform to.
> >
> > Signed-off-by: Grant Seltzer <grantseltzer@gmail.com>
>
> Acked-by: Song Liu <songliubraving@fb.com>
>

Applied to bpf-next, thanks. I've fixed the example block comment
indentation (it was off by one space for all but the first line). I
also noticed that vim syntax highlighting were treating '/**' as
something special, so I added escaping: '/\*\*', but it's not easy for
me to serve html from my dev server, so not sure if that renders ok.
At least `make htmldocs` didn't complain.

BTW, as we'll have (hopefully) more contributions to libbpf docs, it
would be nice to integrate `make htmldocs` "check" into
selftests/bpf's Makefile. Can you please see if that's possible?
Ideally we'd only build docs inside Documentation/bpf to speed things
up. Seems like we currently have one error/warning there, so we are in
a pretty good shape already, but playing a proactive defense here
seems like a prudent approach?


> > ---
> >  .../bpf/libbpf/libbpf_naming_convention.rst   | 40 +++++++++++++++++++
> >  1 file changed, 40 insertions(+)
> >
> > diff --git a/Documentation/bpf/libbpf/libbpf_naming_convention.rst b/Documentation/bpf/libbpf/libbpf_naming_convention.rst
> > index 9c68d5014ff1..5f42f172987a 100644
> > --- a/Documentation/bpf/libbpf/libbpf_naming_convention.rst
> > +++ b/Documentation/bpf/libbpf/libbpf_naming_convention.rst
> > @@ -150,6 +150,46 @@ mirror of the mainline's version of libbpf for a stand-alone build.
> >  However, all changes to libbpf's code base must be upstreamed through
> >  the mainline kernel tree.
> >
> > +
> > +API documentation convention
> > +============================
> > +
> > +The libbpf API is documented via comments above definitions in
> > +header files. These comments can be rendered by doxygen and sphinx
> > +for well organized html output. This section describes the
> > +convention in which these comments should be formated.
> > +
> > +Here is an example from btf.h:
> > +
> > +.. code-block:: c
> > +
> > +        /**
> > +        * @brief **btf__new()** creates a new instance of a BTF object from the raw
> > +        * bytes of an ELF's BTF section
> > +        * @param data raw bytes
> > +        * @param size number of bytes passed in `data`
> > +        * @return new BTF object instance which has to be eventually freed with
> > +        * **btf__free()**
> > +        *
> > +        * On error, error-code-encoded-as-pointer is returned, not a NULL. To extract
> > +        * error code from such a pointer `libbpf_get_error()` should be used. If
> > +        * `libbpf_set_strict_mode(LIBBPF_STRICT_CLEAN_PTRS)` is enabled, NULL is
> > +        * returned on error instead. In both cases thread-local `errno` variable is
> > +        * always set to error code as well.
> > +        */
> > +
> > +The comment must start with a block comment of the form '/**'.
> > +
> > +The documentation always starts with a @brief directive. This line is a short
> > +description about this API. It starts with the name of the API, denoted in bold
> > +like so: **api_name**. Please include an open and close parenthesis if this is a
> > +function. Follow with the short description of the API. A longer form description
> > +can be added below the last directive, at the bottom of the comment.
> > +
> > +Parameters are denoted with the @param directive, there should be one for each
> > +parameter. If this is a function with a non-void return, use the @return directive
> > +to document it.
> > +
> >  License
> >  -------------------
> >
> > --
> > 2.31.1
> >
Grant Seltzer Richman Oct. 7, 2021, 11:27 p.m. UTC | #3 
On Wed, Oct 6, 2021 at 1:29 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Mon, Oct 4, 2021 at 6:00 PM Song Liu <song@kernel.org> wrote:
> >
> > On Mon, Oct 4, 2021 at 4:51 PM grantseltzer <grantseltzer@gmail.com> wrote:
> > >
> > > From: Grant Seltzer <grantseltzer@gmail.com>
> > >
> > > This adds a section to the documentation for libbpf
> > > naming convention which describes how to document
> > > API features in libbpf, specifically the format of
> > > which API doc comments need to conform to.
> > >
> > > Signed-off-by: Grant Seltzer <grantseltzer@gmail.com>
> >
> > Acked-by: Song Liu <songliubraving@fb.com>
> >
>
> Applied to bpf-next, thanks. I've fixed the example block comment
> indentation (it was off by one space for all but the first line). I
> also noticed that vim syntax highlighting were treating '/**' as
> something special, so I added escaping: '/\*\*', but it's not easy for
> me to serve html from my dev server, so not sure if that renders ok.
> At least `make htmldocs` didn't complain.
>
> BTW, as we'll have (hopefully) more contributions to libbpf docs, it
> would be nice to integrate `make htmldocs` "check" into
> selftests/bpf's Makefile. Can you please see if that's possible?
> Ideally we'd only build docs inside Documentation/bpf to speed things
> up. Seems like we currently have one error/warning there, so we are in
> a pretty good shape already, but playing a proactive defense here
> seems like a prudent approach?

That's a good idea, I will work on this.

>
>
> > > ---
> > >  .../bpf/libbpf/libbpf_naming_convention.rst   | 40 +++++++++++++++++++
> > >  1 file changed, 40 insertions(+)
> > >
> > > diff --git a/Documentation/bpf/libbpf/libbpf_naming_convention.rst b/Documentation/bpf/libbpf/libbpf_naming_convention.rst
> > > index 9c68d5014ff1..5f42f172987a 100644
> > > --- a/Documentation/bpf/libbpf/libbpf_naming_convention.rst
> > > +++ b/Documentation/bpf/libbpf/libbpf_naming_convention.rst
> > > @@ -150,6 +150,46 @@ mirror of the mainline's version of libbpf for a stand-alone build.
> > >  However, all changes to libbpf's code base must be upstreamed through
> > >  the mainline kernel tree.
> > >
> > > +
> > > +API documentation convention
> > > +============================
> > > +
> > > +The libbpf API is documented via comments above definitions in
> > > +header files. These comments can be rendered by doxygen and sphinx
> > > +for well organized html output. This section describes the
> > > +convention in which these comments should be formated.
> > > +
> > > +Here is an example from btf.h:
> > > +
> > > +.. code-block:: c
> > > +
> > > +        /**
> > > +        * @brief **btf__new()** creates a new instance of a BTF object from the raw
> > > +        * bytes of an ELF's BTF section
> > > +        * @param data raw bytes
> > > +        * @param size number of bytes passed in `data`
> > > +        * @return new BTF object instance which has to be eventually freed with
> > > +        * **btf__free()**
> > > +        *
> > > +        * On error, error-code-encoded-as-pointer is returned, not a NULL. To extract
> > > +        * error code from such a pointer `libbpf_get_error()` should be used. If
> > > +        * `libbpf_set_strict_mode(LIBBPF_STRICT_CLEAN_PTRS)` is enabled, NULL is
> > > +        * returned on error instead. In both cases thread-local `errno` variable is
> > > +        * always set to error code as well.
> > > +        */
> > > +
> > > +The comment must start with a block comment of the form '/**'.
> > > +
> > > +The documentation always starts with a @brief directive. This line is a short
> > > +description about this API. It starts with the name of the API, denoted in bold
> > > +like so: **api_name**. Please include an open and close parenthesis if this is a
> > > +function. Follow with the short description of the API. A longer form description
> > > +can be added below the last directive, at the bottom of the comment.
> > > +
> > > +Parameters are denoted with the @param directive, there should be one for each
> > > +parameter. If this is a function with a non-void return, use the @return directive
> > > +to document it.
> > > +
> > >  License
> > >  -------------------
> > >
> > > --
> > > 2.31.1
> > >
diff mbox series

Patch

diff --git a/Documentation/bpf/libbpf/libbpf_naming_convention.rst b/Documentation/bpf/libbpf/libbpf_naming_convention.rst
index 9c68d5014ff1..5f42f172987a 100644
--- a/Documentation/bpf/libbpf/libbpf_naming_convention.rst
+++ b/Documentation/bpf/libbpf/libbpf_naming_convention.rst
@@ -150,6 +150,46 @@  mirror of the mainline's version of libbpf for a stand-alone build.
 However, all changes to libbpf's code base must be upstreamed through
 the mainline kernel tree.
 
+
+API documentation convention
+============================
+
+The libbpf API is documented via comments above definitions in
+header files. These comments can be rendered by doxygen and sphinx
+for well organized html output. This section describes the
+convention in which these comments should be formated.
+
+Here is an example from btf.h:
+
+.. code-block:: c
+
+        /**
+        * @brief **btf__new()** creates a new instance of a BTF object from the raw
+        * bytes of an ELF's BTF section
+        * @param data raw bytes
+        * @param size number of bytes passed in `data`
+        * @return new BTF object instance which has to be eventually freed with
+        * **btf__free()**
+        *
+        * On error, error-code-encoded-as-pointer is returned, not a NULL. To extract
+        * error code from such a pointer `libbpf_get_error()` should be used. If
+        * `libbpf_set_strict_mode(LIBBPF_STRICT_CLEAN_PTRS)` is enabled, NULL is
+        * returned on error instead. In both cases thread-local `errno` variable is
+        * always set to error code as well.
+        */
+
+The comment must start with a block comment of the form '/**'.
+
+The documentation always starts with a @brief directive. This line is a short
+description about this API. It starts with the name of the API, denoted in bold
+like so: **api_name**. Please include an open and close parenthesis if this is a
+function. Follow with the short description of the API. A longer form description
+can be added below the last directive, at the bottom of the comment.
+
+Parameters are denoted with the @param directive, there should be one for each
+parameter. If this is a function with a non-void return, use the @return directive
+to document it.
+
 License
 -------------------