[igt-dev] [PATCH i-g-t] tests/i915/i915_pm_dc: Changing sysfs open attribute to O_RDONLY

[Petri Latvala]

On Thu, Sep 30, 2021 at 03:42:17PM +0300, Bhatt, Jigar wrote:
> 
> 
> -----Original Message-----
> From: Latvala, Petri <petri.latvala at intel.com>
> Sent: Thursday, September 30, 2021 6:07 PM
> To: Bhatt, Jigar <jigar.bhatt at intel.com>
> Cc: igt-dev at lists.freedesktop.org; Varide, Nischal <nischal.varide at intel.com>
> Subject: Re: [igt-dev] [PATCH i-g-t] tests/i915/i915_pm_dc: Changing sysfs open attribute to O_RDONLY
> 
> On Thu, Sep 30, 2021 at 05:50:50PM +0530, Jigar Bhatt wrote:
> > Opening /sys/module/drm_kms_helper/parameters/ in read only mode to
> > avoid permission issue in dc9-dpms test.
> >
> > Signed-off-by: Jigar Bhatt <jigar.bhatt at intel.com>
> > ---
> >  tests/i915/i915_pm_dc.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/tests/i915/i915_pm_dc.c b/tests/i915/i915_pm_dc.c index
> > d70ee060..4c9b871c 100644
> > --- a/tests/i915/i915_pm_dc.c
> > +++ b/tests/i915/i915_pm_dc.c
> > @@ -419,7 +419,7 @@ static void setup_dc9_dpms(data_t *data, int
> > dc_target)  {
> >       int prev_dc, sysfs_fd;
> >
> > -     igt_require((sysfs_fd = open(KMS_HELPER, O_RDWR)) >= 0);
> > +     igt_require((sysfs_fd = open(KMS_HELPER, O_RDONLY)) >= 0);
> >       kms_poll_saved_state = igt_sysfs_get_boolean(sysfs_fd, "poll");
> >       igt_sysfs_set_boolean(sysfs_fd, "poll", KMS_POLL_DISABLE);
> >       close(sysfs_fd);
> > @@ -449,7 +449,7 @@ static void kms_poll_state_restore(int sig)  {
> >       int sysfs_fd;
> >
> > -     sysfs_fd = open(KMS_HELPER, O_WRONLY);
> > +     sysfs_fd = open(KMS_HELPER, O_RDONLY);
> >       if (sysfs_fd >= 0) {
> >               igt_sysfs_set_boolean(sysfs_fd, "poll", kms_poll_saved_state);
> 
> Shouldn't this stay WRONLY?
> 
> 
> --
> Petri Latvala
> 
> Hi Petri,
> Here since we are opening a directory, it needs to open as read-only.
> This is the same scenario with other test cases too example,
> igt_aux.c     igt_require((power_dir = open("/sys/power", O_RDONLY)) >= 0);

Ah yes, it's the directory, not the file. ENOCOFFEE...

Reviewed-by: Petri Latvala <petri.latvala at intel.com>


> 
> 
> thanks and regards,
> Jigar Bhatt
> >               close(sysfs_fd);
> > --
> > 2.25.1
> >