On 17/07/2020 23:55, Tony Ambardar wrote:
> The bpftool sources include code to walk file trees, but use multiple
> frameworks to do so: nftw and fts. While nftw conforms to POSIX/SUSv3 and
> is widely available, fts is not conformant and less common, especially on
> non-glibc systems. The inconsistent framework usage hampers maintenance
> and portability of bpftool, in particular for embedded systems.
>
> Standardize code usage by rewriting one fts-based function to use nftw and
> clean up some related function warnings by extending use of "const char *"
> arguments. This change helps in building bpftool against musl for OpenWrt.
>
> Also fix an unsafe call to dirname() by duplicating the string to pass,
> since some implementations may directly alter it. The same approach is
> used in libbpf.c.
>
> Signed-off-by: Tony Ambardar <Tony.Ambardar@xxxxxxxxx>
> ---
>
> V3:
> * clarify dirname() path copy in commit message
> * fix whitespace and rearrange comment for clarity
> * drop unnecessary initializers, rebalance Christmas tree
> * fixup error message and drop others not previously present
> * simplify malloc() + memset() -> calloc() and check for mem errors
>
> V2:
> * use _GNU_SOURCE to pull in getpagesize(), getline(), nftw() definitions
> * use "const char *" in open_obj_pinned() and open_obj_pinned_any()
> * make dirname() safely act on a string copy
>
> ---
> tools/bpf/bpftool/common.c | 132 +++++++++++++++++++++----------------
> tools/bpf/bpftool/main.h | 4 +-
> 2 files changed, 78 insertions(+), 58 deletions(-)
>
> diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
> index 29f4e7611ae8..2ecfafcd01df 100644
> --- a/tools/bpf/bpftool/common.c
> +++ b/tools/bpf/bpftool/common.c
> int build_pinned_obj_table(struct pinned_obj_table *tab,
> enum bpf_obj_type type)
> {
[...]
> while ((mntent = getmntent(mntfile))) {
[...]
> - while ((ftse = fts_read(fts))) {
[...]
> + if (nftw(path, do_build_table_cb, nopenfd, flags) == -1)
> + break;
Sorry I missed that on the previous reviews; but I think a simple break
out of the loop changes the previous behaviour, we should instead
"return -1" from build_pinned_obj_table() if nftw() returns -1, as we
were doing so far.
Looks good otherwise.
> }
> fclose(mntfile);
> return 0;
