Ton of warnings in atomisp driver #1036

nathanchance · 2020-05-27T05:07:30Z

I am going to be sending a series for these, I will post a link here once it has been sent.

nathanchance · 2020-05-27T07:12:42Z

Series sent: https://lore.kernel.org/lkml/20200527071150.3381228-1-natechancellor@gmail.com/

Clang warns: ../drivers/staging/media/atomisp/pci/sh_css_sp.c:1039:23: warning: address of 'binary->in_frame_info' will always evaluate to 'true' [-Wpointer-bool-conversion] } else if (&binary->in_frame_info) { ~~ ~~~~~~~~^~~~~~~~~~~~~ in_frame_info is not a pointer so if binary is not NULL, in_frame_info's address cannot be NULL. Change this to an else since it will always be evaluated as one. While we are here, clean up this if block. The contents of both if blocks are the same but a check against "stage == 0" is added when ISP2401 is defined. USE_INPUT_SYSTEM_VERSION_2401 is only defined when isp2401_system_global.h is included, which only happens when ISP2401. In other words, USE_INPUT_SYSTEM_VERSION_2401 always requires ISP2401 to be defined so the '#ifndef ISP2401' makes no sense. Remove that part of the block to simplify everything. Link: ClangBuiltLinux#1036 Reported-by: kbuild test robot <lkp@intel.com> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Clang warns: ../drivers/staging/media/atomisp/pci/atomisp_v4l2.c:1097:3: warning: variable 'count' is incremented both in the loop header and in the loop body [-Wfor-loop-analysis] count++; ^ This was probably unintentional, remove it. Link: ClangBuiltLinux#1036 Reported-by: kbuild test robot <lkp@intel.com> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

…tension Clang warns: ../drivers/staging/media/atomisp/pci/sh_css.c:8537:14: warning: address of 'pipe->output_stage' will always evaluate to 'true' [-Wpointer-bool-conversion] if (&pipe->output_stage) ~~ ~~~~~~^~~~~~~~~~~~ ../drivers/staging/media/atomisp/pci/sh_css.c:8545:14: warning: address of 'pipe->vf_stage' will always evaluate to 'true' [-Wpointer-bool-conversion] if (&pipe->vf_stage) ~~ ~~~~~~^~~~~~~~ output_stage and vf_stage are pointers in the middle of a struct, their addresses cannot be NULL if pipe is not NULL and pipe is already checked for NULL in this function. Simplify this if block. Link: ClangBuiltLinux#1036 Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Clang warns: drivers/staging/media/atomisp/pci/atomisp_cmd.c:4278:17: warning: address of 'config->info' will always evaluate to 'true' [-Wpointer-bool-conversion] if (!&config->info) { ~ ~~~~~~~~^~~~ config cannot be NULL because it comes from an ioctl, which ensures that the user is not giving us an invalid pointer through copy_from_user. If config is not NULL, info cannot be NULL. Remove this check. Link: ClangBuiltLinux#1036 Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Clang warns: drivers/staging/media/atomisp/pci/isp/kernels/xnr/xnr_3.0/ia_css_xnr3.host.c:129:35: warning: implicit conversion from 'unsigned long' to 'int32_t' (aka 'int') changes value from 18446744073709543424 to -8192 [-Wconstant-conversion] return MAX(MIN(isp_strength, 0), -XNR_BLENDING_SCALE_FACTOR); ~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~ XNR_BLENDING_SCALE_FACTOR is BIT(13), or 8192, which will easily fit into a signed 32-bit integer. However, it is an unsigned long, which means that negating it is the same as subtracting that value from ULONG_MAX + 1, which causes it to be larger than a signed 32-bit integer so it gets implicitly converted. We can avoid this by using the variable isp_scale, which holds the value of XNR_BLENDING_SCALE_FACTOR already, where the implicit conversion from unsigned long to s32 already happened. If that were to ever overflow, clang would warn: https://godbolt.org/z/EeSxLG Link: ClangBuiltLinux#1036 Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

Clang warns: drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c:1707:64: warning: implicit conversion from enumeration type 'const enum ia_css_frame_format' to different enumeration type 'enum atomisp_input_format' [-Wenum-conversion] binary_supports_input_format(xcandidate, req_in_info->format)); ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~^~~~~~ As it turns out, binary_supports_input_format only asserts that xcandidate is not NULL and just returns true so this call is never actually made. There are other functions that are called that assert info is not NULL so this function actually serves no purpose. Remove it. It can be brought back if needed later. Link: ClangBuiltLinux#1036 Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

nathanchance · 2020-06-03T07:56:07Z

My and Arnd's fixes are now in -next: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/log/drivers/staging/media/atomisp?h=next-20200602