Re: [PATCH bpf-next v2 12/16] selftests: xsk: remove cleanup at end of program

On Thu, Aug 19, 2021 at 11:43 AM Maciej Fijalkowski
[off-list ref] wrote:

On Tue, Aug 17, 2021 at 11:27:25AM +0200, Magnus Karlsson wrote:

quoted

From: Magnus Karlsson <magnus.karlsson@intel.com>

Remove the cleanup right before the program/process exits as this will
trigger the cleanup without us having to write or maintain any
code. The application is not a library, so let us benefit from that.

Not a fan of that, I'd rather keep things tidy, but you're right that
dropping this logic won't hurt us.

My strategy here was that all functions should clean up themselves and
be tidy, the exception to that being the main function as if it
exists, the program is gone and libc will clean up the allocations for
us. Maybe a bit pragmatic, but I do prefer less code in this case. On
the other hand,
I do not have any strong objections to keeping the code. Just think it
is unnecessary. But if we hide the allocations in a function, then I
would need to deallocate them later for it to look clean (according to
the above logic). Maybe that will improve readabilty. Will try it out.

/Magnus

quoted

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 tools/testing/selftests/bpf/xdpxceiver.c | 29 +++++-------------------
 1 file changed, 6 insertions(+), 23 deletions(-)

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 8ff24472ef1e..c1bb03e0ca07 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c

@@ -1041,28 +1041,24 @@ static void run_pkt_test(int mode, int type)
 int main(int argc, char **argv)
 {
      struct rlimit _rlim = { RLIM_INFINITY, RLIM_INFINITY };
-     bool failure = false;
      int i, j;

      if (setrlimit(RLIMIT_MEMLOCK, &_rlim))
              exit_with_error(errno);

-     for (int i = 0; i < MAX_INTERFACES; i++) {
+     for (i = 0; i < MAX_INTERFACES; i++) {
              ifdict[i] = malloc(sizeof(struct ifobject));
              if (!ifdict[i])
                      exit_with_error(errno);

              ifdict[i]->ifdict_index = i;
              ifdict[i]->xsk_arr = calloc(2, sizeof(struct xsk_socket_info *));
-             if (!ifdict[i]->xsk_arr) {
-                     failure = true;
-                     goto cleanup;
-             }
+             if (!ifdict[i]->xsk_arr)
+                     exit_with_error(errno);
+
              ifdict[i]->umem_arr = calloc(2, sizeof(struct xsk_umem_info *));
-             if (!ifdict[i]->umem_arr) {
-                     failure = true;
-                     goto cleanup;
-             }
+             if (!ifdict[i]->umem_arr)
+                     exit_with_error(errno);
      }

      setlocale(LC_ALL, "");

@@ -1081,19 +1077,6 @@ int main(int argc, char **argv)
              }
      }

-cleanup:
-     for (int i = 0; i < MAX_INTERFACES; i++) {
-             if (ifdict[i]->ns_fd != -1)
-                     close(ifdict[i]->ns_fd);
-             free(ifdict[i]->xsk_arr);
-             free(ifdict[i]->umem_arr);
-             free(ifdict[i]);
-     }
-
-     if (failure)
-             exit_with_error(errno);
-
      ksft_exit_pass();
-
      return 0;
 }
--

2.29.0