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@redhat.com>
---
 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);
 
             if (candidate && STREQ(candidate->iScsiName, name)) {
-                found = true;
+                hostScsiTopologyTarget = target;
                 break;
             }
         }
     }
 
-    if (!found || !hostScsiTopologyTarget)
+    if (!hostScsiTopologyTarget)
         goto cleanup;
 
     if (!hostScsiTopologyTarget->lun) {
-- 
2.48.1

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@redhat.com>
> ---
>  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@redhat.com>