Re: [PATCH 15/20] drm: simplify drm_*_set_unique()

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Aug 29, 2014 at 12:12:41PM +0200, David Herrmann wrote:
> Lets use kasprintf() to avoid pre-allocating the buffer. This is really
> nothing to optimize for speed and the input is trusted, so kasprintf() is
> just fine.
> 
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_pci.c      | 30 ++++++++----------------------
>  drivers/gpu/drm/drm_platform.c | 31 ++++++++-----------------------
>  2 files changed, 16 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
> index 020cfd9..8efea6b 100644
> --- a/drivers/gpu/drm/drm_pci.c
> +++ b/drivers/gpu/drm/drm_pci.c
> @@ -129,31 +129,17 @@ static int drm_get_pci_domain(struct drm_device *dev)
>  
>  static int drm_pci_set_busid(struct drm_device *dev, struct drm_master *master)
>  {
> -	int len, ret;
> -	master->unique_len = 40;
> -	master->unique_size = master->unique_len;
> -	master->unique = kmalloc(master->unique_size, GFP_KERNEL);
> -	if (master->unique == NULL)
> +	master->unique = kasprintf(GFP_KERNEL, "pci:%04x:%02x:%02x.%d",
> +					drm_get_pci_domain(dev),
> +					dev->pdev->bus->number,
> +					PCI_SLOT(dev->pdev->devfn),
> +					PCI_FUNC(dev->pdev->devfn));
> +	if (!master->unique)
>  		return -ENOMEM;
>  
> -
> -	len = snprintf(master->unique, master->unique_len,
> -		       "pci:%04x:%02x:%02x.%d",
> -		       drm_get_pci_domain(dev),
> -		       dev->pdev->bus->number,
> -		       PCI_SLOT(dev->pdev->devfn),
> -		       PCI_FUNC(dev->pdev->devfn));
> -
> -	if (len >= master->unique_len) {
> -		DRM_ERROR("buffer overflow");
> -		ret = -EINVAL;
> -		goto err;
> -	} else
> -		master->unique_len = len;
> -
> +	master->unique_len = strlen(master->unique);
> +	master->unique_size = master->unique_len + 1;
>  	return 0;
> -err:
> -	return ret;
>  }
>  
>  int drm_pci_set_unique(struct drm_device *dev,
> diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c
> index d5b76f1..0c09ddd 100644
> --- a/drivers/gpu/drm/drm_platform.c
> +++ b/drivers/gpu/drm/drm_platform.c
> @@ -70,35 +70,20 @@ err_free:
>  
>  static int drm_platform_set_busid(struct drm_device *dev, struct drm_master *master)
>  {
> -	int len, ret, id;
> -
> -	master->unique_len = 13 + strlen(dev->platformdev->name);
> -	master->unique_size = master->unique_len;
> -	master->unique = kmalloc(master->unique_len + 1, GFP_KERNEL);
> -
> -	if (master->unique == NULL)
> -		return -ENOMEM;
> +	int id;
>  
>  	id = dev->platformdev->id;
> -
> -	/* if only a single instance of the platform device, id will be
> -	 * set to -1.. use 0 instead to avoid a funny looking bus-id:
> -	 */
> -	if (id == -1)
> +	if (id < 0)
>  		id = 0;
>  
> -	len = snprintf(master->unique, master->unique_len,
> -			"platform:%s:%02d", dev->platformdev->name, id);
> -
> -	if (len > master->unique_len) {
> -		DRM_ERROR("Unique buffer overflowed\n");
> -		ret = -EINVAL;
> -		goto err;
> -	}
> +	master->unique = kasprintf(GFP_KERNEL, "platform:%s:%02d",
> +						dev->platformdev->name, id);
> +	if (!master->unique)
> +		return -ENOMEM;
>  
> +	master->unique_len = strlen(master->unique);
> +	master->unique_size = master->unique_len;
>  	return 0;
> -err:
> -	return ret;
>  }
>  
>  static struct drm_bus drm_platform_bus = {
> -- 
> 2.1.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch