* [igt-dev] [PATCH i-g-t] lib/drmtest: Change do_ioctl_err() macro to function
@ 2023-04-28 11:52 Zbigniew Kempczyński
2023-04-28 16:24 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
0 siblings, 1 reply; 4+ messages in thread
From: Zbigniew Kempczyński @ 2023-04-28 11:52 UTC (permalink / raw)
To: igt-dev
Chris enlightened me after I did some review do_ioctl_err() macro
is a little bit problematic when it catches unexpected error (debug
output isn't much descriptive). As we plan to use it widely in xe
uapi verification lets make error callstack more readable.
Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
---
lib/drmtest.c | 12 ++++++++++++
lib/drmtest.h | 10 +++-------
2 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/lib/drmtest.c b/lib/drmtest.c
index c91a914257..759e889874 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -681,3 +681,15 @@ void igt_require_xe(int fd)
{
igt_require(is_xe_device(fd));
}
+
+void do_ioctl_err(int fd, unsigned long ioc, void *ioc_data, int err)
+{
+ int ret;
+
+ ret = igt_ioctl(fd, ioc, ioc_data);
+ igt_assert_f(ret == -1,
+ "ioctl should fail with -1 but returned %d\n", ret);
+ igt_assert_f(errno == err,
+ "errno fail - returned: %d, expected: %d\n", errno, err);
+ errno = 0;
+}
diff --git a/lib/drmtest.h b/lib/drmtest.h
index 3c88b85c6f..792996dd28 100644
--- a/lib/drmtest.h
+++ b/lib/drmtest.h
@@ -153,13 +153,9 @@ bool is_intel_device(int fd);
* @ioc_data: data pointer for the ioctl operation
* @err: value to expect in errno
*
- * This macro wraps drmIoctl() and uses igt_assert to check that it fails,
- * returning a particular value in errno.
+ * This function wraps drmIoctl() and uses igt_assert to check that it fails,
+ * verifying expected errno in @err.
*/
-#define do_ioctl_err(fd, ioc, ioc_data, err) do { \
- igt_assert_eq(igt_ioctl((fd), (ioc), (ioc_data)), -1); \
- igt_assert_eq(errno, err); \
- errno = 0; \
-} while (0)
+void do_ioctl_err(int fd, unsigned long ioc, void *ioc_data, int err);
#endif /* DRMTEST_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* [igt-dev] [PATCH i-g-t] lib/drmtest: Change do_ioctl_err() macro to function
@ 2023-05-09 6:39 Zbigniew Kempczyński
2023-05-09 8:01 ` Kamil Konieczny
0 siblings, 1 reply; 4+ messages in thread
From: Zbigniew Kempczyński @ 2023-05-09 6:39 UTC (permalink / raw)
To: igt-dev
Chris enlightened me after I did some review do_ioctl_err() macro
is a little bit problematic when it catches unexpected error (debug
output isn't much descriptive). As we plan to use it widely in xe
uapi verification lets make error callstack more readable.
Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
---
lib/drmtest.c | 12 ++++++++++++
lib/drmtest.h | 10 +++-------
2 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/lib/drmtest.c b/lib/drmtest.c
index c91a914257..759e889874 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -681,3 +681,15 @@ void igt_require_xe(int fd)
{
igt_require(is_xe_device(fd));
}
+
+void do_ioctl_err(int fd, unsigned long ioc, void *ioc_data, int err)
+{
+ int ret;
+
+ ret = igt_ioctl(fd, ioc, ioc_data);
+ igt_assert_f(ret == -1,
+ "ioctl should fail with -1 but returned %d\n", ret);
+ igt_assert_f(errno == err,
+ "errno fail - returned: %d, expected: %d\n", errno, err);
+ errno = 0;
+}
diff --git a/lib/drmtest.h b/lib/drmtest.h
index 3c88b85c6f..792996dd28 100644
--- a/lib/drmtest.h
+++ b/lib/drmtest.h
@@ -153,13 +153,9 @@ bool is_intel_device(int fd);
* @ioc_data: data pointer for the ioctl operation
* @err: value to expect in errno
*
- * This macro wraps drmIoctl() and uses igt_assert to check that it fails,
- * returning a particular value in errno.
+ * This function wraps drmIoctl() and uses igt_assert to check that it fails,
+ * verifying expected errno in @err.
*/
-#define do_ioctl_err(fd, ioc, ioc_data, err) do { \
- igt_assert_eq(igt_ioctl((fd), (ioc), (ioc_data)), -1); \
- igt_assert_eq(errno, err); \
- errno = 0; \
-} while (0)
+void do_ioctl_err(int fd, unsigned long ioc, void *ioc_data, int err);
#endif /* DRMTEST_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [igt-dev] [PATCH i-g-t] lib/drmtest: Change do_ioctl_err() macro to function
2023-05-09 6:39 [igt-dev] [PATCH i-g-t] " Zbigniew Kempczyński
@ 2023-05-09 8:01 ` Kamil Konieczny
0 siblings, 0 replies; 4+ messages in thread
From: Kamil Konieczny @ 2023-05-09 8:01 UTC (permalink / raw)
To: igt-dev
Hi Zbigniew,
On 2023-05-09 at 08:39:21 +0200, Zbigniew Kempczyński wrote:
> Chris enlightened me after I did some review do_ioctl_err() macro
> is a little bit problematic when it catches unexpected error (debug
> output isn't much descriptive). As we plan to use it widely in xe
> uapi verification lets make error callstack more readable.
It is up to you if you would like to have a macro or a function,
you can improve macro to have more descriptive prints.
>
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> ---
> lib/drmtest.c | 12 ++++++++++++
> lib/drmtest.h | 10 +++-------
> 2 files changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index c91a914257..759e889874 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -681,3 +681,15 @@ void igt_require_xe(int fd)
> {
> igt_require(is_xe_device(fd));
> }
> +
Please write here description.
> +void do_ioctl_err(int fd, unsigned long ioc, void *ioc_data, int err)
> +{
> + int ret;
> +
> + ret = igt_ioctl(fd, ioc, ioc_data);
> + igt_assert_f(ret == -1,
> + "ioctl should fail with -1 but returned %d\n", ret);
> + igt_assert_f(errno == err,
> + "errno fail - returned: %d, expected: %d\n", errno, err);
--------------------- ^ --- ^
Maybe "ioctl mismatch error:" ?
> + errno = 0;
> +}
> diff --git a/lib/drmtest.h b/lib/drmtest.h
> index 3c88b85c6f..792996dd28 100644
> --- a/lib/drmtest.h
> +++ b/lib/drmtest.h
> @@ -153,13 +153,9 @@ bool is_intel_device(int fd);
> * @ioc_data: data pointer for the ioctl operation
> * @err: value to expect in errno
> *
> - * This macro wraps drmIoctl() and uses igt_assert to check that it fails,
> - * returning a particular value in errno.
> + * This function wraps drmIoctl() and uses igt_assert to check that it fails,
> + * verifying expected errno in @err.
> */
If this is a function then descritpion should be moved to .c file
> -#define do_ioctl_err(fd, ioc, ioc_data, err) do { \
> - igt_assert_eq(igt_ioctl((fd), (ioc), (ioc_data)), -1); \
You can improve macro with temporary var for ret and change
asserts into igt_assert_f.
Second idea would be to have
bool __do_ioctl_err
call it from macro and fail if not true.
Regards,
Kamil
> - igt_assert_eq(errno, err); \
> - errno = 0; \
> -} while (0)
> +void do_ioctl_err(int fd, unsigned long ioc, void *ioc_data, int err);
>
> #endif /* DRMTEST_H */
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-05-09 8:01 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-28 11:52 [igt-dev] [PATCH i-g-t] lib/drmtest: Change do_ioctl_err() macro to function Zbigniew Kempczyński
2023-04-28 16:24 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
-- strict thread matches above, loose matches on Subject: below --
2023-05-09 6:39 [igt-dev] [PATCH i-g-t] " Zbigniew Kempczyński
2023-05-09 8:01 ` Kamil Konieczny
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox