Re: [PATCH v5 2/4] drm: Expose wedge recovery methods

[Raag Jadav <raag.jadav@intel.com>]

On Thu, Sep 19, 2024 at 10:38:51AM +0300, Jani Nikula wrote:
> On Thu, 19 Sep 2024, Raag Jadav <raag.jadav@intel.com> wrote:
> > On Tue, Sep 17, 2024 at 10:49:07AM +0300, Jani Nikula wrote:
> >> On Tue, 17 Sep 2024, Raag Jadav <raag.jadav@intel.com> wrote:
> >> > Now that we have device wedged event in place, add wedge_recovery sysfs
> >> > attribute which will expose recovery methods supported by the DRM device.
> >> > This is useful for userspace consumers in cases where the device supports
> >> > multiple recovery methods which can be used as fallbacks.
> >> >
> >> > $ cat /sys/class/drm/card0/wedge_recovery
> >> > rebind
> >> > bus-reset
> >> > reboot
> >> >
> >> > Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> >> > ---
> >> >  drivers/gpu/drm/drm_sysfs.c | 23 +++++++++++++++++++++++
> >> >  1 file changed, 23 insertions(+)
> >> >
> >> > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> >> > index fb3bbb6adcd1..b88cdbfa3b5e 100644
> >> > --- a/drivers/gpu/drm/drm_sysfs.c
> >> > +++ b/drivers/gpu/drm/drm_sysfs.c
> >> > @@ -36,6 +36,8 @@
> >> >  #define to_drm_minor(d) dev_get_drvdata(d)
> >> >  #define to_drm_connector(d) dev_get_drvdata(d)
> >> >  
> >> > +extern const char *const wedge_recovery_opts[];
> >> 
> >> Data is not an interface. Please add a function for this.
> >
> > For a single user?
> 
> Yes.
> 
> Well, you kind of have two, and both places need to do bounds checking
> on indexing the array. You also need to do bounds checking on the string
> manipulation, you can't just strcat and assume it'll be all right.

Which would be true if we were to receive an unknown string. Here we sorta
know it offhand so we're not gonna shoot in our foot :D

Anyway, would you prefer strlcat instead?

Raag