Re: [PATCH] drm/i915: Clean up ancient doc comments for i915_ioc32.c

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 14 Feb 2018, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> As befitting a file dedicated to the mistakes of the past,
>
> drivers/gpu/drm/i915/i915_ioc32.c:2: warning: Cannot understand  * \file i915_ioc32.c
>  on line 2 - I thought it was a doc line
> drivers/gpu/drm/i915/i915_ioc32.c:82: warning: Function parameter or member 'filp' not described in 'i915_compat_ioctl'
> drivers/gpu/drm/i915/i915_ioc32.c:82: warning: Function parameter or member 'cmd' not described in 'i915_compat_ioctl'
> drivers/gpu/drm/i915/i915_ioc32.c:82: warning: Function parameter or member 'arg' not described in 'i915_compat_ioctl'
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_ioc32.c | 38 ++++++++++++++++----------------------
>  1 file changed, 16 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_ioc32.c b/drivers/gpu/drm/i915/i915_ioc32.c
> index 97f3a5640289..73599be641e7 100644
> --- a/drivers/gpu/drm/i915/i915_ioc32.c
> +++ b/drivers/gpu/drm/i915/i915_ioc32.c
> @@ -1,11 +1,6 @@
> -/**
> - * \file i915_ioc32.c
> - *
> +/*
>   * 32-bit ioctl compatibility routines for the i915 DRM.
>   *
> - * \author Alan Hourihane <alanh@fairlite.demon.co.uk>
> - *
> - *
>   * Copyright (C) Paul Mackerras 2005
>   * Copyright (C) Alan Hourihane 2005
>   * All Rights Reserved.
> @@ -28,6 +23,8 @@
>   * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
>   * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
>   * IN THE SOFTWARE.
> + *
> + * Author: Alan Hourihane <alanh@fairlite.demon.co.uk>
>   */
>  #include <linux/compat.h>
>  
> @@ -55,9 +52,9 @@ static int compat_i915_getparam(struct file *file, unsigned int cmd,
>  		return -EFAULT;
>  
>  	request = compat_alloc_user_space(sizeof(*request));
> -	if (!access_ok(VERIFY_WRITE, request, sizeof(*request))
> -	    || __put_user(req32.param, &request->param)
> -	    || __put_user((void __user *)(unsigned long)req32.value,
> +	if (!access_ok(VERIFY_WRITE, request, sizeof(*request)) ||
> +	    __put_user(req32.param, &request->param) ||
> +	    __put_user((void __user *)(unsigned long)req32.value,
>  			  &request->value))
>  		return -EFAULT;
>  
> @@ -70,30 +67,27 @@ static drm_ioctl_compat_t *i915_compat_ioctls[] = {
>  };
>  
>  /**
> + * i915_compat_ioctl - handle the mistakes of the past
> + * @filp: the file pointer
> + * @cmd: the ioctl command (and encoded flags)
> + * @arg: the ioctl argument (from userspace)
> + *
>   * Called whenever a 32-bit process running under a 64-bit kernel
>   * performs an ioctl on /dev/dri/card<n>.
> - *
> - * \param filp file pointer.
> - * \param cmd command.
> - * \param arg user argument.
> - * \return zero on success or negative number on failure.
>   */
>  long i915_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  {
>  	unsigned int nr = DRM_IOCTL_NR(cmd);
> -	drm_ioctl_compat_t *fn = NULL;
> -	int ret;
> +	drm_ioctl_compat_t *fn;
>  
>  	if (nr < DRM_COMMAND_BASE || nr >= DRM_COMMAND_END)
>  		return drm_compat_ioctl(filp, cmd, arg);
>  
> +	fn = NULL;
>  	if (nr < DRM_COMMAND_BASE + ARRAY_SIZE(i915_compat_ioctls))
>  		fn = i915_compat_ioctls[nr - DRM_COMMAND_BASE];
> +	if (fn)
> +		return fn(filp, cmd, arg);

Not really fond of code juggling in a patch that's supposed to be about
doc comment updates.

But if you do this, you might just as well move the if (fn) check within
the preceding if block, and get rid of the fn = NULL initialization
altogheter.

BR,
Jani.


>  
> -	if (fn != NULL)
> -		ret = (*fn) (filp, cmd, arg);
> -	else
> -		ret = drm_ioctl(filp, cmd, arg);
> -
> -	return ret;
> +	return drm_ioctl(filp, cmd, arg);
>  }

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx