[PATCH 02/10] drm/exynos: ipp: fix incorrect format specifiers in debug messages
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
>>> ---
>>> 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
[PATCH 02/10] drm/exynos: ipp: fix incorrect format specifiers in debug messages
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
>> ---
>> 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.
Thanks,
Inki Dae
>
> Best regards,
> Krzysztof
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc"
> in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
[PATCH 02/10] drm/exynos: ipp: fix incorrect format specifiers in debug messages
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
> ---
> 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?
Best regards,
Krzysztof
[PATCH 02/10] drm/exynos: ipp: fix incorrect format specifiers in debug messages
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
---
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);
spin_lock_init(>lock);
platform_set_drvdata(pdev, ctx);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
index 7aecd23..5d20da8 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
@@ -1723,7 +1723,7 @@ static int gsc_probe(struct platform_device *pdev)
return ret;
}
- DRM_DEBUG_KMS("id[%d]ippdrv[0x%x]\n", ctx->id, (int)ippdrv);
+ DRM_DEBUG_KMS("id[%d]ippdrv[%p]\n", ctx->id, ippdrv);
mutex_init(>lock);
platform_set_drvdata(pdev, ctx);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index 67d2423..95eeb91 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -208,7 +208,7 @@ static struct exynos_drm_ippdrv *ipp_find_drv_by_handle(u32
prop_id)
* e.g PAUSE state, queue buf, command control.
*/
list_for_each_entry(ippdrv, _drm_ippdrv_list, drv_list) {
- DRM_DEBUG_KMS("count[%d]ippdrv[0x%x]\n", count++, (int)ippdrv);
+ DRM_DEBUG_KMS("count[%d]ippdrv[%p]\n", count++, ippdrv);
mutex_lock(>cmd_lock);
list_for_each_entry(c_node, >cmd_list, list) {
@@ -388,8 +388,8 @@ int exynos_drm_ipp_set_property(struct drm_device *drm_dev,
void *data,
}
property->prop_id = ret;
- DRM_DEBUG_KMS("created prop_id[%d]cmd[%d]ippdrv[0x%x]\n",
- property->prop_id, property->cmd, (int)ippdrv);
+ DRM_DEBUG_KMS("created prop_id[%d]cmd[%d]ippdrv[%p]\n",
+ property->prop_id, property->cmd, ippdrv);
/* stored property information and ippdrv in private data */
c_node->property = *property;
@@ -518,7 +518,7 @@ static int ipp_put_mem_node(struct drm_device *drm_dev,
{
int i;
- DRM_DEBUG_KMS("node[0x%x]\n", (int)m_node);
+ DRM_DEBUG_KMS("node[%p]\n", m_node);
if (!m_node) {
DRM_ERROR("invalid dequeue node.\n");
@@ -562,7 +562,7 @@ static struct drm_exynos_ipp_mem_node
m_node->buf_id = qbuf->buf_id;
INIT_LIST_HEAD(_node->list);
- DRM_DEBUG_KMS("m_node[0x%x]ops_id[%d]\n", (int)m_node, qbuf->ops_id);
+ DRM_DEBUG_KMS("m_node[%p]ops_id[%d]\n", m_node, qbuf->ops_id);
DRM_DEBUG_KMS("prop_id[%d]buf_id[%d]\n", qbuf->prop_id, m_node->buf_id);
for_each_ipp_planar(i) {
@@ -582,8 +582,8 @@ static struct drm_exynos_ipp_mem_node
buf_info->handles[i] = qbuf->handle[i];
buf_info->base[i] = *addr;
- DRM_DEBUG_KMS("i[%d]base[0x%x]hd[0x%lx]\n", i,
- buf_info->base[i], buf_info->handles[i]);
+ DRM_DEBUG_KMS("i[%d]base[%pad]hd[0x%lx]\n", i,
+ _info->base[i], buf_info->handles[i]);
}
}
@@ -664,7 +664,7 @@ static void ipp_put_event(struct drm_exynos_ipp_cmd_node
*c_node,
mutex_lock(_node->event_lock);
list_for_each_entry_safe(e, te, _node->event_list, base.link) {
- DRM_DEBUG_KMS("count[%d]e[0x%x]\n", count++, (int)e);
+ DRM_DEBUG_KMS("count[%d]e[%p]\n", count++, e);
/*
* qbuf == NULL condition means all event deletion.
@@ -755,7 +755,7 @@ static struct drm_exynos_ipp_mem_node
/* find memory node from memory list */
list_for_each_entry(m_node, head, list) {
- DRM_DEBUG_KMS("count[%d]m_node[0x%x]\n", count++, (int)m_node);
+ DRM_DEBUG_KMS("count[%d]m_node[%p]\n", count++, m_node);
/* compare buffer id */
if (m_node->buf_id == qbuf->buf_id)
@@ -772,7 +772,7 @@ static int ipp_set_mem_node(struct exynos_drm_ippdrv
*ippdrv,
struct exynos_drm_ipp_ops *ops = NULL;
int ret = 0;
- DRM_DEBUG_KMS("node[0x%x]\n", (int)m_node);
+ DRM_DEBUG_KMS("node[%p]\n",
