On Tue, Sep 14, 2021 at 1:17 PM grantseltzer <grantseltzer@xxxxxxxxx> wrote:
>
> From: Grant Seltzer <grantseltzer@xxxxxxxxx>
>
> This adds comments above five functions in btf.h which document
> their uses. These comments are of a format that doxygen and sphinx
> can pick up and render. These are rendered by libbpf.readthedocs.org
>
> Signed-off-by: Grant Seltzer <grantseltzer@xxxxxxxxx>
> ---
> tools/lib/bpf/btf.h | 37 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
>
> diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
> index 4a711f990904..05e06f0136e3 100644
> --- a/tools/lib/bpf/btf.h
> +++ b/tools/lib/bpf/btf.h
> @@ -1,5 +1,6 @@
> /* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
> /* Copyright (c) 2018 Facebook */
> +/*! \file */
What's the purpose of this? Is it some sort of description for the entire file?
>
> #ifndef __LIBBPF_BTF_H
> #define __LIBBPF_BTF_H
> @@ -30,11 +31,47 @@ enum btf_endianness {
> BTF_BIG_ENDIAN = 1,
> };
>
> +/**
> + * @brief **btf__free()** frees all data of a BTF object.
> + * @param btf BTF object to free
> + * @return void
agreed to drop this one
> + */
> LIBBPF_API void btf__free(struct btf *btf);
>
> +/**
> + * @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 length of raw bytes
reads a bit weird, bytes don't have "length". "Number of bytes passed
in `data`"?
> + * @return new instance of BTF object which has to be eventually freed
> + * with **btf__free()**
> + */
> LIBBPF_API struct btf *btf__new(const void *data, __u32 size);
> +
> +/**
> + * @brief **btf__new_split()** create a new instance of a BTF object from
> + * the provided raw data bytes. It takes another BTF instance, **base_btf**,
> + * which serves as a base BTF, which is extended by types in a newly created
> + * BTF instance.
> + * @param data raw bytes
> + * @param size length of raw bytes
> + * @param base_btf the base btf object
> + * @return struct btf *
I didn't think I had to leave a suggestion under every such empty @return...
BTW, return documentation is finally a good place where we should
document quirky libbpf error returning behavior. Something like this:
```
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.
```
We should have this remark under every pointer-returning API which has
this error-code-as-ptr logic (not all APIs do).
> + */
> LIBBPF_API struct btf *btf__new_split(const void *data, __u32 size, struct btf *base_btf);
> +
> +/**
> + * @brief **btf__new_empty()** creates an unpopulated BTF object.
We can add "Use `btf__add_*()` to populate such BTF object.
> + * @return struct btf *
another not described @return
> + */
> LIBBPF_API struct btf *btf__new_empty(void);
> +
> +/**
> + * @brief **btf__new_empty_split()** creates an unpopulated
> + * BTF object from an ELF BTF section except with a base BTF
> + * on top of which split BTF should be based.
If *base_btf* is NULL, `btf__new_empty_split()` is equivalent to
`btf__new_empty()` and creates non-split BTF.
> + * @return struct btf *
and one more
> + */
> LIBBPF_API struct btf *btf__new_empty_split(struct btf *base_btf);
>
> LIBBPF_API struct btf *btf__parse(const char *path, struct btf_ext **btf_ext);
> --
> 2.31.1
>
