linux-kernel - Re: [PATCH] drm/irq: Modify the return value type of drm_irq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7a06d5ba-27c6-0762-662c-fee1f8ddbc2e@suse.de>
Date:   Mon, 2 Nov 2020 13:48:52 +0100
From:   Thomas Zimmermann <tzimmermann@...e.de>
To:     Tian Tao <tiantao6@...ilicon.com>,
        maarten.lankhorst@...ux.intel.com, mripard@...nel.org,
        airlied@...ux.ie, daniel@...ll.ch, dri-devel@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/irq: Modify the return value type of
 drm_irq_uninstall

Hi

Am 02.11.20 um 13:38 schrieb Tian Tao:
> There is no driver to use the return value of drm_irq_uninstal,
> so modify the return value type of drm_irq_uninstal to void.
> 
> Signed-off-by: Tian Tao <tiantao6@...ilicon.com>
> ---
>  drivers/gpu/drm/drm_irq.c | 13 ++++++-------
>  include/drm/drm_irq.h     |  2 +-
>  2 files changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 7537a3d..45e6471 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -166,14 +166,14 @@ EXPORT_SYMBOL(drm_irq_install);
>   * Returns:
>   * Zero on success or a negative error code on failure.
>   */
> -int drm_irq_uninstall(struct drm_device *dev)
> +void drm_irq_uninstall(struct drm_device *dev)
>  {
>  	unsigned long irqflags;
>  	bool irq_enabled;
>  	int i;
>  
>  	if (!dev->irq_enabled || !dev)
> -		return 0;
> +		return;
>  
>  	irq_enabled = dev->irq_enabled;
>  	dev->irq_enabled = false;
> @@ -200,8 +200,8 @@ int drm_irq_uninstall(struct drm_device *dev)
>  		spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
>  	}
>  
> -	if (!irq_enabled)
> -		return -EINVAL;
> +	if (!drm_WARN_ON(dev, !irq_enabled))
> +		return;
>  
>  	DRM_DEBUG("irq=%d\n", dev->irq);
>  
> @@ -213,7 +213,6 @@ int drm_irq_uninstall(struct drm_device *dev)
>  
>  	free_irq(dev->irq, dev);
>  
> -	return 0;
>  }
>  EXPORT_SYMBOL(drm_irq_uninstall);
>  
> @@ -250,10 +249,10 @@ int drm_legacy_irq_control(struct drm_device *dev, void *data,
>  		return ret;
>  	case DRM_UNINST_HANDLER:
>  		mutex_lock(&dev->struct_mutex);
> -		ret = drm_irq_uninstall(dev);
> +		drm_irq_uninstall(dev);

Oh, there actually is a user of this result! I grep'ed for this but
didn't see it. I'm sorry for misleading you here.

This is ioctl code and who which program depends on it.So we cannot
actually drop the result code.

I'll just ack your original patch, or you could add the managed
interface that I described and convert hibmc to it. Your choice, let me
know.

Best regards
Thomas

>  		mutex_unlock(&dev->struct_mutex);
>  
> -		return ret;
> +		return 0;
>  	default:
>  		return -EINVAL;
>  	}
> diff --git a/include/drm/drm_irq.h b/include/drm/drm_irq.h
> index d77f6e6..d9f6ec0 100644
> --- a/include/drm/drm_irq.h
> +++ b/include/drm/drm_irq.h
> @@ -27,6 +27,6 @@
>  struct drm_device;
>  
>  int drm_irq_install(struct drm_device *dev, int irq);
> -int drm_irq_uninstall(struct drm_device *dev);
> +void drm_irq_uninstall(struct drm_device *dev);
>  
>  #endif
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer

Download attachment "OpenPGP_0x680DC11D530B7A23.asc" of type "application/pgp-keys" (4202 bytes)

Download attachment "OpenPGP_signature" of type "application/pgp-signature" (496 bytes)