[igt-dev] [PATCH i-g-t 3/4] lib/kselftests: return non-zero on open(kmsg) failure

[Tales]

Hello, Kamil,

Em qua., 3 de ago. de 2022 às 14:11, Kamil Konieczny
<kamil.konieczny at linux.intel.com> escreveu:
>
> Hi Tales,
>
> On 2022-08-03 at 02:26:53 -0300, Tales Aparecida wrote:
> > Previously igt_kselftest_begin() always returned 0.
> > Return non-zero if failed to open kmsg, instead.
> >
> > Signed-off-by: Tales Aparecida <tales.aparecida at gmail.com>
> > ---
> >  lib/igt_kmod.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> > index bde0461a..63636243 100644
> > --- a/lib/igt_kmod.c
> > +++ b/lib/igt_kmod.c
> > @@ -933,6 +933,8 @@ int igt_kselftest_begin(struct igt_kselftest *tst)
> >       igt_require(err == 0 || err == -ENOENT);
> >
> >       tst->kmsg = open("/dev/kmsg", O_RDONLY | O_NONBLOCK);
> > +     if (tst->kmsg < 0)
> > +             return 1;
>
>
> This is used everywhere in dumping error messages, for example
> in igt_kselftest_execute() there is:
>         if (err)
>                 kmsg_dump(tst->kmsg);
>
> so leaving this as is may be dangerous. So either fdup on stderr
> or use
>         igt_reguire(test->kmsg >= 0);
>
> Regards,
> Kamil
>
> >
> >       return 0;
> >  }
> > --
> > 2.37.0
> >

That's a great point! I guess I prefer the latter, using igt_require,
for it is simpler and I believe errors when opening kmsg are too
seldom to justify anything too complex.
With that said, I think that the function won't need to return, after all.

Thanks for the review!