Re: [PATCH v5 1/2] selftests/dmabuf-heap: conform test to TAP format output

[Shuah Khan]

On 3/4/24 23:08, Muhammad Usama Anjum wrote:

Conform the layout, informational and status messages to TAP. No
functional change is intended other than the layout of output messages.

Reviewed-by: T.J. Mercier <tjmerc...@google.com>
Signed-off-by: Muhammad Usama Anjum <usama.an...@collabora.com>
---
Changes since v4:
- close fds correctly with code changes added in v3

Chanages since v3:
- abort test-case instead of exiting if heap/mem allocation fails
- Correct test_alloc_zeroed() test case in case of failure

Changes since v2:
- Minor improvements in test_alloc_zeroed() results

Changes since v1:
- Update some more error handling code

t would be nice to improve the error messages in addition to
changing them over to TAP format. Please see below:

---
  .../selftests/dmabuf-heaps/dmabuf-heap.c      | 246 +++++++-----------
  1 file changed, 101 insertions(+), 145 deletions(-)

diff --git a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c 
b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c
index 890a8236a8ba7..e7bd03e0af2ea 100644
--- a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c
+++ b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c
@@ -15,6 +15,7 @@
  #include <linux/dma-buf.h>
  #include <linux/dma-heap.h>
  #include <drm/drm.h>
+#include "../kselftest.h"

#define DEVPATH "/dev/dma_heap" @@ -90,14 +91,13 @@ static int dmabuf_heap_open(char *name)

        char buf[256];

ret = snprintf(buf, 256, "%s/%s", DEVPATH, name);

-       if (ret < 0) {
-               printf("snprintf failed!\n");
-               return ret;
-       }
+       if (ret < 0)
+               ksft_exit_fail_msg("snprintf failed!\n");

Why not include the return value in the message?

fd = open(buf, O_RDWR);

        if (fd < 0)
-               printf("open %s failed!\n", buf);
+               ksft_exit_fail_msg("open %s failed: %s\n", buf, 
strerror(errno));
+
        return fd;
  }

@@ -140,7 +140,7 @@ static int dmabuf_sync(int fd, int start_stop) #define ONE_MEG (1024 * 1024) -static int test_alloc_and_import(char *heap_name)

+static void test_alloc_and_import(char *heap_name)
  {
        int heap_fd = -1, dmabuf_fd = -1, importer_fd = -1;
        uint32_t handle = 0;
@@ -148,27 +148,19 @@ static int test_alloc_and_import(char *heap_name)
        int ret;

heap_fd = dmabuf_heap_open(heap_name);

-       if (heap_fd < 0)
-               return -1;

- printf(" Testing allocation and importing: ");

+       ksft_print_msg("Testing allocation and importing:\n");
        ret = dmabuf_heap_alloc(heap_fd, ONE_MEG, 0, &dmabuf_fd);
        if (ret) {
-               printf("FAIL (Allocation Failed!)\n");
-               ret = -1;
-               goto out;
+               ksft_test_result_fail("FAIL (Allocation Failed!)\n");

Same here/

+               return;
        }
+
        /* mmap and write a simple pattern */
-       p = mmap(NULL,
-                ONE_MEG,
-                PROT_READ | PROT_WRITE,
-                MAP_SHARED,
-                dmabuf_fd,
-                0);
+       p = mmap(NULL, ONE_MEG, PROT_READ | PROT_WRITE, MAP_SHARED, dmabuf_fd, 
0);
        if (p == MAP_FAILED) {
-               printf("FAIL (mmap() failed)\n");
-               ret = -1;
-               goto out;
+               ksft_test_result_fail("FAIL (mmap() failed)\n");

Same here and the rest of the changes in this patch.

thanks,
-- Shuah