Resending as plain-text to rest of list.
Steve
On Wed, Mar 20, 2019 at 7:58 AM Stephen Hines <srhi...@google.com> wrote:
>
> Why are there 2 different enums for this same thing at all? By casting, you
> are reducing type safety in the kernel, which can cause bugs later (should
> the two enums diverge in encoding). In my opinion, the proper solution is to
> remove one of the enums or provide an explicit helper that does the
> conversion (along with assertions for handling any unexpected cases). The
> helper function should not be doing a direct cast, since a bug could change
> the integer value of one (or both) of these enums so that they don't match up.
>
> Thanks,
> Steve
>
> On Wed, Mar 20, 2019 at 2:37 AM Koenig, Christian <christian.koe...@amd.com>
> wrote:
>>
>> Am 20.03.19 um 05:34 schrieb Nathan Chancellor:
>> > On Wed, Mar 20, 2019 at 01:31:27AM +0000, Pan, Xinhui wrote:
>> >> these two enumerated types are same for now. both of them might change in
>> >> the future.
>> >>
>> >> I have not used clang, but would .block_id = (int)head->block fix your
>> >> warning? If such change is acceptable, I can make one then.
>> >>
>> >> Thanks
>> >> xinhui
>> >>
>> >>
>> >> -----Original Message-----
>> >> From: Nathan Chancellor <natechancel...@gmail.com>
>> >> Sent: 2019年3月20日 8:54
>> >> To: Deucher, Alexander <alexander.deuc...@amd.com>; Koenig, Christian
>> >> <christian.koe...@amd.com>; Zhou, David(ChunMing) <david1.z...@amd.com>;
>> >> Pan, Xinhui <xinhui....@amd.com>
>> >> Cc: amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
>> >> linux-ker...@vger.kernel.org; clang-built-li...@googlegroups.com
>> >> Subject: Clang warning in drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
>> >>
>> >> Hi all,
>> >>
>> >> The introduction of this file in commit dbd249c24427 ("drm/amdgpu: add
>> >> amdgpu_ras.c to support ras (v2)") introduces the following Clang
>> >> warnings:
>> >>
>> >> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:544:23: warning: implicit
>> >> conversion from enumeration type 'enum amdgpu_ras_block' to different
>> >> enumeration type 'enum ta_ras_block' [-Wenum-conversion]
>> >> .block_id = head->block,
>> >> ~~~~~~^~~~~
>> >> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:545:24: warning: implicit
>> >> conversion from enumeration type 'enum amdgpu_ras_error_type' to
>> >> different enumeration type 'enum ta_ras_error_type' [-Wenum-conversion]
>> >> .error_type = head->type,
>> >> ~~~~~~^~~~
>> >> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:549:23: warning: implicit
>> >> conversion from enumeration type 'enum amdgpu_ras_block' to different
>> >> enumeration type 'enum ta_ras_block' [-Wenum-conversion]
>> >> .block_id = head->block,
>> >> ~~~~~~^~~~~
>> >> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:550:24: warning: implicit
>> >> conversion from enumeration type 'enum amdgpu_ras_error_type' to
>> >> different enumeration type 'enum ta_ras_error_type' [-Wenum-conversion]
>> >> .error_type = head->type,
>> >> ~~~~~~^~~~
>> >> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:650:26: warning: implicit
>> >> conversion from enumeration type 'enum amdgpu_ras_block' to different
>> >> enumeration type 'enum ta_ras_block' [-Wenum-conversion]
>> >> .block_id = info->head.block,
>> >> ~~~~~~~~~~~^~~~~
>> >> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:651:35: warning: implicit
>> >> conversion from enumeration type 'enum amdgpu_ras_error_type' to
>> >> different enumeration type 'enum ta_ras_error_type' [-Wenum-conversion]
>> >> .inject_error_type = info->head.type,
>> >> ~~~~~~~~~~~^~~~
>> >> 6 warnings generated.
>> >>
>> >> Normally, I would sent a fix for this myself but I am not entirely sure
>> >> why these two enumerated types exist when one would do since they have
>> >> the same values minus the prefix. In fact, the ta_ras_{block,error_type}
>> >> values are never used aside from being defined. Some clarification would
>> >> be appreciated.
>> >>
>> >> Thank you,
>> >> Nathan
>> > Hi Xinhui,
>> >
>> > Yes, explicitly casting these six spots to int would resolve this
>> > warning.
>>
>> Another question is if it is such a good idea to just silence the warning?
>>
>> Maybe add a amdgpu_ras_error_to_ta() helper to do this casting?
>>
>> When the enums drift away from each other then we can still add warnings
>> to that helper to make sure we don't accidentally cast invalid values
>> around.
>>
>> Regards,
>> Christian.
>>
>> >
>> > Thank you for the quick response!
>> > Nathan
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "Clang Built Linux" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to clang-built-linux+unsubscr...@googlegroups.com.
>> To post to this group, send email to clang-built-li...@googlegroups.com.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/clang-built-linux/63518f1f-b808-77b0-aac6-ee1ece669c4b%40amd.com.
>> For more options, visit https://groups.google.com/d/optout.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
