[PATCH] drm/amdkfd: Add curly braces around idr_for_each_entry_continue loop

[Kuehling, Felix]

On 2019-03-05 6:20 a.m., Michel Dänzer wrote:
> From: Michel Dänzer <michel.daenzer at amd.com>
>
> The compiler pointed out that one if block unintentionally wasn't part
> of the loop:
>
> In file included from ./include/linux/kernfs.h:14,
>                   from ./include/linux/sysfs.h:16,
>                   from ./include/linux/kobject.h:20,
>                   from ./include/linux/device.h:16,
>                   from ./include/linux/node.h:18,
>                   from ./include/linux/memory.h:19,
>                   from drivers/gpu/drm//amd/amdgpu/../amdkfd/kfd_events.c:30:
> drivers/gpu/drm//amd/amdgpu/../amdkfd/kfd_events.c: In function ‘kfd_signal_reset_event’:
> ./include/linux/idr.h:212:2: warning: this ‘for’ clause does not guard... [-Wmisleading-indentation]
>    for ((entry) = idr_get_next((idr), &(id));   \
>    ^~~
> drivers/gpu/drm//amd/amdgpu/../amdkfd/kfd_events.c:1038:3: note: in expansion of macro ‘idr_for_each_entry_continue’
>     idr_for_each_entry_continue(&p->event_idr, ev, id)
>     ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm//amd/amdgpu/../amdkfd/kfd_events.c:1043:4: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the ‘for’
>      if (ev->type == KFD_EVENT_TYPE_MEMORY &&
>      ^~
>
> Fixes: "drm/amdkfd: add RAS ECC event support"
> Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>

Reviewed-by: Felix Kuehling <Felix.Kuehling at amd.com>


> ---
>
> This is one reason why I think it's better to always use curly braces
> around multiple lines, even if it happens to be a single statement.
>
>   drivers/gpu/drm/amd/amdkfd/kfd_events.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_events.c b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
> index 97c984684973..6e1d41c5bf86 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
> @@ -1035,7 +1035,7 @@ void kfd_signal_reset_event(struct kfd_dev *dev)
>   	hash_for_each_rcu(kfd_processes_table, temp, p, kfd_processes) {
>   		mutex_lock(&p->event_mutex);
>   		id = KFD_FIRST_NONSIGNAL_EVENT_ID;
> -		idr_for_each_entry_continue(&p->event_idr, ev, id)
> +		idr_for_each_entry_continue(&p->event_idr, ev, id) {
>   			if (ev->type == KFD_EVENT_TYPE_HW_EXCEPTION) {
>   				ev->hw_exception_data = hw_exception_data;
>   				set_event(ev);
> @@ -1045,6 +1045,7 @@ void kfd_signal_reset_event(struct kfd_dev *dev)
>   				ev->memory_exception_data = memory_exception_data;
>   				set_event(ev);
>   			}
> +		}
>   		mutex_unlock(&p->event_mutex);
>   	}
>   	srcu_read_unlock(&kfd_processes_srcu, idx);