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

["Ser, Simon" <simon.ser@intel.com>]

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
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev