On Thu, 24 Oct 2019 15:23:41 +0200, Jiri Olsa wrote:
> The bpftool interface stays the same, but now it's possible
> to run it over BTF raw data, like:
>
> $ bpftool btf dump file /sys/kernel/btf/vmlinux
> [1] INT '(anon)' size=4 bits_offset=0 nr_bits=32 encoding=(none)
> [2] INT 'long unsigned int' size=8 bits_offset=0 nr_bits=64 encoding=(none)
> [3] CONST '(anon)' type_id=2
My knee jerk reaction would be to implement a new keyword, like:
$ bpftool btf dump rawfile /sys/kernel/btf/vmlinux
Or such. But perhaps the auto-detection is the standard way of dealing
with different formats in the compiler world. Regardless if anyone has
an opinion one way or the other please share!!
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
> v2 changes:
> - added is_btf_raw to find out which btf__parse_* function to call
> - changed labels and error propagation in btf__parse_raw
> - drop the err initialization, which is not needed under this change
The code looks good, thanks for the changes! One question below..
> diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
> index 9a9376d1d3df..a7b8bf233cf5 100644
> --- a/tools/bpf/bpftool/btf.c
> +++ b/tools/bpf/bpftool/btf.c
> +static bool is_btf_raw(const char *file)
> +{
> + __u16 magic = 0;
> + int fd;
> +
> + fd = open(file, O_RDONLY);
> + if (fd < 0)
> + return false;
> +
> + read(fd, &magic, sizeof(magic));
> + close(fd);
> + return magic == BTF_MAGIC;
Isn't it suspicious to read() 2 bytes into an u16 and compare to a
constant like endianness doesn't matter? Quick grep doesn't reveal
BTF_MAGIC being endian-aware..
> +}
> +
> static int do_dump(int argc, char **argv)
> {
> struct btf *btf = NULL;
> @@ -465,7 +516,11 @@ static int do_dump(int argc, char **argv)
> }
> NEXT_ARG();
> } else if (is_prefix(src, "file")) {
> - btf = btf__parse_elf(*argv, NULL);
> + if (is_btf_raw(*argv))
> + btf = btf__parse_raw(*argv);
> + else
> + btf = btf__parse_elf(*argv, NULL);
> if (IS_ERR(btf)) {
> err = PTR_ERR(btf);
> btf = NULL;
