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>
> ---
>  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));

I think we've been trying to standardize on aligning parameters on
subsequent lines with the first parameter on the first line.

[...]
> +	master->unique_len = strlen(master->unique);
> +	master->unique_size = master->unique_len + 1;

[...]
> diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c
[...]
>  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;

Perhaps collapse all of the above into:

	int id = (dev->platformdev->id < 0) ? 0 : dev->platformdev->id;

? Not that it matters much. I suspect we could easily remove all traces
of this particular function in a next step.

> -	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;

unique_size is weird. It seems to me like it should always be unique_len
+ 1. Why drm_platform_bus should be special escapes me. Also, after this
patch it seems to be completely unused, so perhaps we should just drop
it.

All of those comments can either be addressed in a separate patch (or
ignored), though, so:

Reviewed-by: Thierry Reding <treding@nvidia.com>