On Fri, Feb 5, 2021 at 5:42 AM Giuliano Procida <gprocida@xxxxxxxxxx> wrote:
>
> Many operations in the libelf API return a pointer to a user-provided
> struct (on success) or NULL (on failure).
>
> There are a couple of places in btf_elf__write where both structs and
> pointers to the same structs are used. Holding on to the pointers
> raises ownership and lifetime issues unncessarily and the code is
typo: unnecessarily
> cleaner with only a single access path for these data.
>
> The code now treats the returned pointers as booleans.
>
> Signed-off-by: Giuliano Procida <gprocida@xxxxxxxxxx>
> ---
styling nits, but otherwise LGTM
Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> libbtf.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/libbtf.c b/libbtf.c
> index 7bc49ba..ace8896 100644
> --- a/libbtf.c
> +++ b/libbtf.c
> @@ -698,8 +698,7 @@ int32_t btf_elf__add_datasec_type(struct btf_elf *btfe, const char *section_name
>
> static int btf_elf__write(const char *filename, struct btf *btf)
> {
> - GElf_Shdr shdr_mem, *shdr;
> - GElf_Ehdr ehdr_mem, *ehdr;
> + GElf_Ehdr ehdr;
> Elf_Data *btf_data = NULL;
> Elf_Scn *scn = NULL;
> Elf *elf = NULL;
> @@ -727,13 +726,12 @@ static int btf_elf__write(const char *filename, struct btf *btf)
>
> elf_flagelf(elf, ELF_C_SET, ELF_F_DIRTY);
>
> - ehdr = gelf_getehdr(elf, &ehdr_mem);
> - if (ehdr == NULL) {
> + if (!gelf_getehdr(elf, &ehdr)) {
> elf_error("elf_getehdr failed");
> goto out;
> }
>
> - switch (ehdr_mem.e_ident[EI_DATA]) {
> + switch (ehdr.e_ident[EI_DATA]) {
> case ELFDATA2LSB:
> btf__set_endianness(btf, BTF_LITTLE_ENDIAN);
> break;
> @@ -751,10 +749,10 @@ static int btf_elf__write(const char *filename, struct btf *btf)
>
> elf_getshdrstrndx(elf, &strndx);
> while ((scn = elf_nextscn(elf, scn)) != NULL) {
> - shdr = gelf_getshdr(scn, &shdr_mem);
> - if (shdr == NULL)
> + GElf_Shdr shdr;
it's a good style to have an empty line between variable declaration
block and subsequent instructions
> + if (!gelf_getshdr(scn, &shdr))
> continue;
> - char *secname = elf_strptr(elf, strndx, shdr->sh_name);
> + char *secname = elf_strptr(elf, strndx, shdr.sh_name);
> if (strcmp(secname, ".BTF") == 0) {
> btf_data = elf_getdata(scn, btf_data);
> break;
> --
> 2.30.0.478.g8a0d178c01-goog
>
