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

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 tjmercier@google.com Signed-off-by: Muhammad Usama Anjum usama.anjum@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