| Message ID | 20171128170115.43560-1-michal.wajdeczko@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Michal Wajdeczko (2017-11-28 17:01:15) > In commit 3f6ae7b19 ("igt/kms_frontbuffer_tracking: Keep the debugfs > dir around") we introduced custom variant of __igt_debugfs_read function > that fires assert when debugfs returns an error. Replace that assert > with proper error handling to allow use of errors like -ENODEV. Like ENODEV or just ENODEV? Oh, it appears I goofed in c5da0662d1c0 and didn't change the open error from a bool to an int error code. You could also take the opportunity to then return -errno. And here we can debate whether you want to use if (len < 0) { igt_assert_eq(len, -ENODEV); len = 0; } Other than the topic of whether we want to flag any other error here (e.g. one could imagine a surprising EPERM, EACCES or ENFILE), looks ok. > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On Tue, 2017-11-28 at 17:29 +0000, Chris Wilson wrote: > Quoting Michal Wajdeczko (2017-11-28 17:01:15) > > In commit 3f6ae7b19 ("igt/kms_frontbuffer_tracking: Keep the debugfs > > dir around") we introduced custom variant of __igt_debugfs_read function > > that fires assert when debugfs returns an error. Replace that assert > > with proper error handling to allow use of errors like -ENODEV. > > Like ENODEV or just ENODEV? > > Oh, it appears I goofed in c5da0662d1c0 and didn't change the open error > from a bool to an int error code. You could also take the opportunity to > then return -errno. > > And here we can debate whether you want to use > > if (len < 0) { > igt_assert_eq(len, -ENODEV); > len = 0; > } Let's be explicit at this time, can you resend (with the R-b and this change), Michal. I'll then merge this and the kernel patch. Regards, Joonas > > Other than the topic of whether we want to flag any other error here > (e.g. one could imagine a surprising EPERM, EACCES or ENFILE), looks ok. > > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > -Chris > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoting Joonas Lahtinen (2017-12-04 10:45:13) > On Tue, 2017-11-28 at 17:29 +0000, Chris Wilson wrote: > > Quoting Michal Wajdeczko (2017-11-28 17:01:15) > > > In commit 3f6ae7b19 ("igt/kms_frontbuffer_tracking: Keep the debugfs > > > dir around") we introduced custom variant of __igt_debugfs_read function > > > that fires assert when debugfs returns an error. Replace that assert > > > with proper error handling to allow use of errors like -ENODEV. > > > > Like ENODEV or just ENODEV? > > > > Oh, it appears I goofed in c5da0662d1c0 and didn't change the open error > > from a bool to an int error code. You could also take the opportunity to > > then return -errno. > > > > And here we can debate whether you want to use > > > > if (len < 0) { > > igt_assert_eq(len, -ENODEV); > > len = 0; > > } > > Let's be explicit at this time, can you resend (with the R-b and this > change), Michal. I'll then merge this and the kernel patch. And v2 was applied. -Chris
diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c index a068c8a..50019d1 100644 --- a/tests/kms_frontbuffer_tracking.c +++ b/tests/kms_frontbuffer_tracking.c @@ -783,7 +783,8 @@ static bool set_mode_for_params(struct modeset_params *params) static void __debugfs_read(const char *param, char *buf, int len) { len = igt_sysfs_read(drm.debugfs, param, buf, len - 1); - igt_assert(len > 0); + if (len < 0) + len = 0; buf[len] = '\0'; }
In commit 3f6ae7b19 ("igt/kms_frontbuffer_tracking: Keep the debugfs dir around") we introduced custom variant of __igt_debugfs_read function that fires assert when debugfs returns an error. Replace that assert with proper error handling to allow use of errors like -ENODEV. Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> --- tests/kms_frontbuffer_tracking.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)