Re: [RFC bpf-next 3/4] tools/bpf: bpftool, split the function do_dump()

2018-09-19 Thread Prashant Bhole




On 9/20/2018 12:26 AM, Jakub Kicinski wrote:

On Wed, 19 Sep 2018 16:51:42 +0900, Prashant Bhole wrote:

+static int dump_map_elem(int fd, void *key, void *value,
+struct bpf_map_info *map_info, struct btf *btf,
+json_writer_t *btf_wtr)
+{
+   int num_elems = 0;
+
+   if (!bpf_map_lookup_elem(fd, key, value)) {
+   if (json_output) {
+   print_entry_json(map_info, key, value, btf);
+   } else {
+   if (btf) {
+   struct btf_dumper d = {
+   .btf = btf,
+   .jw = btf_wtr,
+   .is_plain_text = true,
+   };
+
+   do_dump_btf(&d, map_info, key, value);
+   } else {
+   print_entry_plain(map_info, key, value);
+   }
+   num_elems++;
+   }
+   goto out;
+   }
+
+   /* lookup error handling */
+   if (map_is_map_of_maps(map_info->type) ||
+   map_is_map_of_progs(map_info->type))
+   goto out;
+


nit: why not just return?  the goto seems to only do a return anyway,
  is this suggested by some coding style?  Is it to help the
  compiler?  I see people do this from time to time..


Thanks for reviewing. I agree, goto and the label isn't needed. I will 
fix it.


-Prashant


[...]


+out:
+   return num_elems;







Re: [RFC bpf-next 3/4] tools/bpf: bpftool, split the function do_dump()

2018-09-19 Thread Jakub Kicinski
On Wed, 19 Sep 2018 16:51:42 +0900, Prashant Bhole wrote:
> +static int dump_map_elem(int fd, void *key, void *value,
> +  struct bpf_map_info *map_info, struct btf *btf,
> +  json_writer_t *btf_wtr)
> +{
> + int num_elems = 0;
> +
> + if (!bpf_map_lookup_elem(fd, key, value)) {
> + if (json_output) {
> + print_entry_json(map_info, key, value, btf);
> + } else {
> + if (btf) {
> + struct btf_dumper d = {
> + .btf = btf,
> + .jw = btf_wtr,
> + .is_plain_text = true,
> + };
> +
> + do_dump_btf(&d, map_info, key, value);
> + } else {
> + print_entry_plain(map_info, key, value);
> + }
> + num_elems++;
> + }
> + goto out;
> + }
> +
> + /* lookup error handling */
> + if (map_is_map_of_maps(map_info->type) ||
> + map_is_map_of_progs(map_info->type))
> + goto out;
> +

nit: why not just return?  the goto seems to only do a return anyway,
 is this suggested by some coding style?  Is it to help the
 compiler?  I see people do this from time to time..

[...]

> +out:
> + return num_elems;