Re: [PATCH 02/10] drm/exynos: ipp: fix incorrect format specifiers in debug messages

[Krzysztof Kozlowski <k.kozlowski@samsung.com>]

On 04.02.2016 11:34, Inki Dae wrote:
> 
> 
> 2016년 02월 04일 11:17에 Krzysztof Kozlowski 이(가) 쓴 글:
>> On 03.02.2016 21:42, Marek Szyprowski wrote:
>>> Drivers should use %p for printing pointers instead of hardcoding them
>>> as hexadecimal integers. This patch fixes compilation warnings on 64bit
>>> architectures.
>>>
>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> ---
>>>  drivers/gpu/drm/exynos/exynos_drm_fimc.c    |  2 +-
>>>  drivers/gpu/drm/exynos/exynos_drm_gsc.c     |  2 +-
>>>  drivers/gpu/drm/exynos/exynos_drm_ipp.c     | 32 ++++++++++++++---------------
>>>  drivers/gpu/drm/exynos/exynos_drm_rotator.c |  2 +-
>>>  4 files changed, 19 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>>> index c747824..8a4f4a0 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>>> @@ -1723,7 +1723,7 @@ static int fimc_probe(struct platform_device *pdev)
>>>  		goto err_put_clk;
>>>  	}
>>>  
>>> -	DRM_DEBUG_KMS("id[%d]ippdrv[0x%x]\n", ctx->id, (int)ippdrv);
>>> +	DRM_DEBUG_KMS("id[%d]ippdrv[%p]\n", ctx->id, ippdrv);
>>
>> I don't oppose the patch itself but I have different concern. First -
>> probably you meant %pK because this is a writeable structure with
>> function pointers.
>> Second - why the ippdrv has to be printed? Is it useful for debugging?
> 
> Marek fixed just compilation warnings on ARM64 so yes it wouldn't need to print out ippdrv address but as other cleanup patch.

Another patch?

There is no point in a flow like this:
1. Make a patch which changes %x to %p.
2. Make a second patch right after the first one which changes %p to %pK.
3. Optionally make a third patch removing %pK entirely.

There are two issues here (and in other places like in patch 3/10) -
64bit compilation warnings and security related issues. Properly fixing
security related issues would fix in the same time the compilation
warnings...

Best regards,
Krzysztof