Re: [igt-dev] [PATCH i-g-t v2] lib/igt_eld: introduce eld_is_supported

Ser, Simon
2019-09-12T12:30:13Z

On Thu, 2019-09-12 at 13:24 +0100, Chris Wilson wrote:
> Quoting Simon Ser (2019-09-12 13:07:18)
> > We've seen cases in which /proc/asound doesn't exist (e.g. with the
> > new SOF
> > framework). We've also seen cases in which no soundcard is exposed
> > by ALSA (see
> > bugzilla link). Last, some audio drivers din't support ELDs (non-
> > Intel
> > drivers). In all of these cases, skipping the tests depending on
> > ELD support
> > makes more sense and makes it clearer what happens.
> >
> > v2: also check that the driver supports ELDs entries in procfs
> >
> > Signed-off-by: Simon Ser <simon.ser@intel.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102370
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> > lib/igt_eld.c | 17 +++++++++++++++++
> > lib/igt_eld.h | 1 +
> > tests/kms_chamelium.c | 2 ++
> > tests/kms_hdmi_inject.c | 2 ++
> > 4 files changed, 22 insertions(+)
> >
> > diff --git a/lib/igt_eld.c b/lib/igt_eld.c
> > index 16c4ac06c6f6..ab4307f1d289 100644
> > --- a/lib/igt_eld.c
> > +++ b/lib/igt_eld.c
> > @@ -26,6 +26,8 @@
> > #include "config.h"
> >
> > #include <dirent.h>
> > +#include <errno.h>
> > +#include <glob.h>
> > #include <stdint.h>
> > #include <stdio.h>
> > #include <string.h>
> > @@ -262,3 +264,18 @@ bool eld_has_igt(void)
> > struct eld_entry eld;
> > return eld_get_igt(&eld);
> > }
> > +
> > +/** eld_is_supported: check whether the ALSA procfs is enabled,
> > audio cards
> > + * are found and ELDs are supported */
> > +bool eld_is_supported(void)
> > +{
> > + glob_t glob_buf;
> > + bool has_elds;
> > +
> > + igt_assert_f(glob("/proc/asound/card*/" ELD_PREFIX "*",
> > + 0, NULL, &glob_buf) == 0,
> > + "glob failed\n");
>
> GLOB_NOSORT ?
>
> While man doesn't explicitly say what the behaviour with an
> uninitialised glob_t is with no GLOB_APPEND set, I'd feel happier if
> glob_t glob_buf = {};
>
> > + has_elds = glob_buf.gl_pathc > 0;
> > + globfree(&glob_buf);
>
> Give us a new line here.

These are good points. Thanks for the review!

> The glob matches the existing fs walk, so
>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris