Hi Shuah,
Thanks for reviewing.
On 3/27/24 2:05 AM, Shuah Khan wrote:
> 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@xxxxxxxxxx>
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@xxxxxxxxxxxxx>
>> ---
>> 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:
I'll update and send the this patch as v6.
>
>> ---
>> .../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
>
--
BR,
Muhammad Usama Anjum
