On Tue, Mar 11, 2025 at 11:48:41 +0100, Jiri Denemark wrote:
> With a specific combination of compiler options gcc reported the
> following bogus warning (I added a context to it to make the issue
> visible):
>
> ../src/esx/esx_vi.c: In function ‘esxVI_LookupHostScsiTopologyLunListByTargetName’:
> ../src/esx/esx_vi.c:4674:32: error: potential null pointer dereference [-Werror=null-dereference]
> 4671 | if (!found || !hostScsiTopologyTarget)
> 4672 | goto cleanup;
> 4673 |
> 4674 | if (!hostScsiTopologyTarget->lun) {
> | ~~~~~~~~~~~~~~~~~~~~~~^~~~~
>
> Most likely this is caused by found and hostScsiTopologyTarget doing
> essentially the same thing as found is true if and only if
> hostScsiTopologyTarget is non-NULL. The found variable is completely
> redundant. Removing it would be enough, but I decided to make the code a
> little bit easier to read by not using the iterator variable directly.
>
> Signed-off-by: Jiri Denemark <jdenemar@xxxxxxxxxx>
> ---
> src/esx/esx_vi.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
> index e2589aa69a..aee649b86e 100644
> --- a/src/esx/esx_vi.c
> +++ b/src/esx/esx_vi.c
> @@ -4616,7 +4616,6 @@ esxVI_LookupHostScsiTopologyLunListByTargetName
> esxVI_HostScsiTopologyInterface *hostScsiInterfaceList = NULL;
> esxVI_HostScsiTopologyInterface *hostScsiInterface = NULL;
> esxVI_HostScsiTopologyTarget *hostScsiTopologyTarget = NULL;
> - bool found = false;
> esxVI_HostInternetScsiTargetTransport *candidate = NULL;
>
> ESX_VI_CHECK_ARG_LIST(hostScsiTopologyLunList);
> @@ -4653,22 +4652,21 @@ esxVI_LookupHostScsiTopologyLunListByTargetName
>
> /* See vSphere API documentation about HostScsiTopologyInterface */
> for (hostScsiInterface = hostScsiInterfaceList;
> - hostScsiInterface && !found;
> + hostScsiInterface && !hostScsiTopologyTarget;
> hostScsiInterface = hostScsiInterface->_next) {
> - for (hostScsiTopologyTarget = hostScsiInterface->target;
> - hostScsiTopologyTarget;
> - hostScsiTopologyTarget = hostScsiTopologyTarget->_next) {
> + esxVI_HostScsiTopologyTarget *target;
> + for (target = hostScsiInterface->target; target; target = target->_next) {
> candidate = esxVI_HostInternetScsiTargetTransport_DynamicCast
> - (hostScsiTopologyTarget->transport);
> + (target->transport);
Consider putting the argument with the function call operator on the
same line as the function name.
>
> if (candidate && STREQ(candidate->iScsiName, name)) {
> - found = true;
> + hostScsiTopologyTarget = target;
> break;
> }
> }
> }
>
> - if (!found || !hostScsiTopologyTarget)
> + if (!hostScsiTopologyTarget)
> goto cleanup;
>
> if (!hostScsiTopologyTarget->lun) {
Reviewed-by: Peter Krempa <pkrempa@xxxxxxxxxx>
