tools: bpftool: Clean up function to dump map entry

The function used to dump a map entry in bpftool is a bit difficult to
follow, as a consequence to earlier refactorings. There is a variable
("num_elems") which does not appear to be necessary, and the error
handling would look cleaner if moved to its own function. Let's clean it
up. No functional change.

v2:
- v1 was erroneously removing the check on fd maps in an attempt to get
  support for outer map dumps. This is already working. Instead, v2
  focuses on cleaning up the dump_map_elem() function, to avoid
  similar confusion in the future.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Andrii Nakryiko <andriin@fb.com>
Link: https://lore.kernel.org/bpf/20200910102652.10509-2-quentin@isovalent.com

diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index bc0071228f882e221b7f5bd4070e8383ac13cc2d..c8159cb4fb1e74e71ee906fc8443211b58319e9f 100644 (file)
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -213,8 +213,9 @@ static void print_entry_json(struct bpf_map_info *info, unsigned char *key,
        jsonw_end_object(json_wtr);
 }
 
-static void print_entry_error(struct bpf_map_info *info, unsigned char *key,
-                             const char *error_msg)
+static void
+print_entry_error_msg(struct bpf_map_info *info, unsigned char *key,
+                     const char *error_msg)
 {
        int msg_size = strlen(error_msg);
        bool single_line, break_names;
@@ -232,6 +233,40 @@ static void print_entry_error(struct bpf_map_info *info, unsigned char *key,
        printf("\n");
 }
 
+static void
+print_entry_error(struct bpf_map_info *map_info, void *key, int lookup_errno)
+{
+       /* For prog_array maps or arrays of maps, failure to lookup the value
+        * means there is no entry for that key. Do not print an error message
+        * in that case.
+        */
+       if (map_is_map_of_maps(map_info->type) ||
+           map_is_map_of_progs(map_info->type))
+               return;
+
+       if (json_output) {
+               jsonw_start_object(json_wtr);   /* entry */
+               jsonw_name(json_wtr, "key");
+               print_hex_data_json(key, map_info->key_size);
+               jsonw_name(json_wtr, "value");
+               jsonw_start_object(json_wtr);   /* error */
+               jsonw_string_field(json_wtr, "error", strerror(lookup_errno));
+               jsonw_end_object(json_wtr);     /* error */
+               jsonw_end_object(json_wtr);     /* entry */
+       } else {
+               const char *msg = NULL;
+
+               if (lookup_errno == ENOENT)
+                       msg = "<no entry>";
+               else if (lookup_errno == ENOSPC &&
+                        map_info->type == BPF_MAP_TYPE_REUSEPORT_SOCKARRAY)
+                       msg = "<cannot read>";
+
+               print_entry_error_msg(map_info, key,
+                                     msg ? : strerror(lookup_errno));
+       }
+}
+
 static void print_entry_plain(struct bpf_map_info *info, unsigned char *key,
                              unsigned char *value)
 {
@@ -713,56 +748,23 @@ 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;
-       int lookup_errno;
-
-       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++;
-               }
-               return num_elems;
+       if (bpf_map_lookup_elem(fd, key, value)) {
+               print_entry_error(map_info, key, errno);
+               return -1;
        }
 
-       /* lookup error handling */
-       lookup_errno = errno;
-
-       if (map_is_map_of_maps(map_info->type) ||
-           map_is_map_of_progs(map_info->type))
-               return 0;
-
        if (json_output) {
-               jsonw_start_object(json_wtr);
-               jsonw_name(json_wtr, "key");
-               print_hex_data_json(key, map_info->key_size);
-               jsonw_name(json_wtr, "value");
-               jsonw_start_object(json_wtr);
-               jsonw_string_field(json_wtr, "error", strerror(lookup_errno));
-               jsonw_end_object(json_wtr);
-               jsonw_end_object(json_wtr);
-       } else {
-               const char *msg = NULL;
-
-               if (lookup_errno == ENOENT)
-                       msg = "<no entry>";
-               else if (lookup_errno == ENOSPC &&
-                        map_info->type == BPF_MAP_TYPE_REUSEPORT_SOCKARRAY)
-                       msg = "<cannot read>";
+               print_entry_json(map_info, key, value, btf);
+       } else if (btf) {
+               struct btf_dumper d = {
+                       .btf = btf,
+                       .jw = btf_wtr,
+                       .is_plain_text = true,
+               };
 
-               print_entry_error(map_info, key,
-                                 msg ? : strerror(lookup_errno));
+               do_dump_btf(&d, map_info, key, value);
+       } else {
+               print_entry_plain(map_info, key, value);
        }
 
        return 0;
@@ -873,7 +875,8 @@ map_dump(int fd, struct bpf_map_info *info, json_writer_t *wtr,
                                err = 0;
                        break;
                }
-               num_elems += dump_map_elem(fd, key, value, info, btf, wtr);
+               if (!dump_map_elem(fd, key, value, info, btf, wtr))
+                       num_elems++;
                prev_key = key;
        }