Address Coverity, Clang static analyzer warnings

* Fix type of num reliable readers (int to int32_t)

* Conversion codes in debug monitor printf formats

* Dead code elimination

* Skipping a test case where SIZE_MAX is assumed > INT32_MAX if
  assumption is false on target platform

* Error handling in os_sockWaitsetNew

* Stick to unsigned in fragment size calculations

  This check is actually guarded by valid_DataFrag and was safe for
  datagrams up to 2GB, but the unintended and implicit conversion to is
  still best eliminated.

* A "server" connection never has an invalid socket in TCP wrapper

* Handle error return from gethostname in SPDP write (CID 248183)

* Handle extended retcodes in dds_strretcode

  CID 248131, introduced by 19aec98b8a

* Remove dead code in ddsrt logging test (CID 248195)

* Validate command-line argument in process test (CID 248117)

* Allow for extremely delayed store in test

  Test is constructed to have the events trigger only at the appropriate
  times, but it does assume that the store to cb_called becomes visible
  prior to the listener callback.  I'm pretty sure that will always be
  the case in practice, but I'm also pretty sure there is no formal
  guarantee without a memory barrier, which mutex_unlock provides.

  CID 248088, 248136, 248177, 253590, 253591, 253593

* Check unsetenv return value in test (CID 248099)

Signed-off-by: Erik Boasson <eb@ilities.com>

src/core/ddsc/src/dds_domain.c

@@ -169,7 +169,6 @@ static dds_return_t dds_domain_init (dds_domain *domain, dds_domainid_t domain_i
   dds_entity_init_complete (&domain->m_entity);
   return DDS_RETCODE_OK;
 
-  rtps_stop (&domain->gv);
 fail_rtps_start:
   if (domain->gv.config.liveliness_monitoring && dds_global.threadmon_count == 1)
     ddsi_threadmon_stop (dds_global.threadmon);

src/core/ddsc/src/dds_entity.c

@@ -403,14 +403,17 @@ dds_return_t dds_delete_impl_pinned (dds_entity *e, enum delete_impl_state delst
     ddsrt_mutex_unlock (&p->m_mutex);
   }
 
-  /* Do some specific deletion when needed.  Bootstrapping and its inverse are always a
-     tricky business, and here it is no different: deleting the pseudo-top-level object
-     tears down all kinds of stuff that is supposed to remain in existence (like the
-     entire platform abstraction) and so it must be the final call.  Thus, we rely on it
-     to call "dds_entity_final_deinit_before_free" and return a special error code. */
+  /* Do some specific deletion when needed */
   ret = dds_entity_deriver_delete (e);
   if (ret == DDS_RETCODE_NO_DATA)
+  {
+    /* Bootstrapping and its inverse are always a tricky business, and here it is no different:
+       deleting the pseudo-top-level object tears down all kinds of stuff that is supposed to
+       remain in existence (like the entire platform abstraction) and so it must be the final
+       call.  Thus, we rely on it to call "dds_entity_final_deinit_before_free" and return a
+       special error code that we don't pass on to the caller. */
     ret = DDS_RETCODE_OK;
+  }
   else if (ret != DDS_RETCODE_OK)
   {
     if (parent_to_delete)
@@ -423,6 +426,8 @@ dds_return_t dds_delete_impl_pinned (dds_entity *e, enum delete_impl_state delst
     dds_free (e);
   }
 
+  assert (ret == DDS_RETCODE_OK);
+  (void) ret;
   return (parent_to_delete != NULL) ? dds_delete_impl_pinned (parent_to_delete, DIS_IMPLICIT) : DDS_RETCODE_OK;
 }
 

src/core/ddsc/src/dds_stream.c

@@ -1696,7 +1696,7 @@ static bool prtf_simple_array (char * __restrict *buf, size_t * __restrict bufsi
         else
         {
           if (i != 0)
-            cont = prtf (buf, bufsize, ",");
+            (void) prtf (buf, bufsize, ",");
           cont = prtf_simple (buf, bufsize, is, type);
           i++;
         }
@@ -1708,7 +1708,7 @@ static bool prtf_simple_array (char * __restrict *buf, size_t * __restrict bufsi
       for (size_t i = 0; cont && i < num; i++)
       {
         if (i != 0)
-          cont = prtf (buf, bufsize, ",");
+          (void) prtf (buf, bufsize, ",");
         cont = prtf_simple (buf, bufsize, is, type);
       }
       break;

src/core/ddsc/tests/entity_hierarchy.c

@@ -366,6 +366,7 @@ CU_Test(ddsc_entity_get_children, null, .init=hierarchy_init, .fini=hierarchy_fi
 /*************************************************************************************************/
 
 /*************************************************************************************************/
+#if SIZE_MAX > INT32_MAX
 CU_Test(ddsc_entity_get_children, invalid_size, .init=hierarchy_init, .fini=hierarchy_fini)
 {
     dds_return_t ret;
@@ -373,6 +374,7 @@ CU_Test(ddsc_entity_get_children, invalid_size, .init=hierarchy_init, .fini=hier
     ret = dds_get_children(g_participant, &child, SIZE_MAX);
     CU_ASSERT_EQUAL_FATAL(ret, DDS_RETCODE_BAD_PARAMETER);
 }
+#endif
 /*************************************************************************************************/
 
 /*************************************************************************************************/

src/core/ddsc/tests/instance_get_key.c

@@ -132,6 +132,7 @@ CU_Test(ddsc_instance_get_key, registered_instance, .init=setup, .fini=teardown)
     ret = dds_instance_get_key(writer, handle, &key_data);
 
     CU_ASSERT_PTR_NOT_NULL_FATAL(key_data.ip);
+    assert (key_data.ip != NULL); /* for the benefit of clang's static analyzer */
     CU_ASSERT_STRING_EQUAL_FATAL(key_data.ip, data.ip);
     CU_ASSERT_EQUAL_FATAL(key_data.port, data.port);
     CU_ASSERT_EQUAL_FATAL(ret, DDS_RETCODE_OK);
@@ -162,6 +163,7 @@ CU_Test(ddsc_instance_get_key, readcondition, .init=setup, .fini=teardown)
     ret = dds_instance_get_key(readcondition, handle, &key_data);
 
     CU_ASSERT_PTR_NOT_NULL_FATAL(key_data.ip);
+    assert (key_data.ip != NULL); /* for the benefit of clang's static analyzer */
     CU_ASSERT_STRING_EQUAL_FATAL(key_data.ip, data.ip);
     CU_ASSERT_EQUAL_FATAL(key_data.port, data.port);
     CU_ASSERT_EQUAL_FATAL(ret, DDS_RETCODE_OK);
@@ -192,6 +194,7 @@ CU_Test(ddsc_instance_get_key, querycondition, .init=setup, .fini=teardown)
     ret = dds_instance_get_key(querycondition, handle, &key_data);
 
     CU_ASSERT_PTR_NOT_NULL_FATAL(key_data.ip);
+    assert (key_data.ip != NULL); /* for the benefit of clang's static analyzer */
     CU_ASSERT_STRING_EQUAL_FATAL(key_data.ip, data.ip);
     CU_ASSERT_EQUAL_FATAL(key_data.port, data.port);
     CU_ASSERT_EQUAL_FATAL(ret, DDS_RETCODE_OK);

src/core/ddsc/tests/listener.c

@@ -727,7 +727,9 @@ CU_Test(ddsc_listener, publication_matched, .init=init_triggering_test, .fini=fi
     CU_ASSERT_EQUAL_FATAL(publication_matched.last_subscription_handle, reader_hdl);
 
     /* Reset the trigger flags. */
+    ddsrt_mutex_lock(&g_mutex);
     cb_called = 0;
+    ddsrt_mutex_unlock(&g_mutex);
 
     /* Un-match the publication by deleting the reader. */
     dds_delete(g_reader);
@@ -789,7 +791,9 @@ CU_Test(ddsc_listener, subscription_matched, .init=init_triggering_test, .fini=f
     CU_ASSERT_EQUAL_FATAL(subscription_matched.last_publication_handle, writer_hdl);
 
     /* Reset the trigger flags. */
+    ddsrt_mutex_lock(&g_mutex);
     cb_called = 0;
+    ddsrt_mutex_unlock(&g_mutex);
 
     /* Un-match the subscription by deleting the writer. */
     dds_delete(g_writer);
@@ -903,8 +907,10 @@ CU_Test(ddsc_listener, data_available, .init=init_triggering_test, .fini=fini_tr
 
     /* Deleting the writer causes unregisters (or dispose+unregister), and those
        should trigger DATA_AVAILABLE as well */
+    ddsrt_mutex_lock(&g_mutex);
     cb_called = 0;
     cb_reader = 0;
+    ddsrt_mutex_unlock(&g_mutex);
     ret = dds_delete (g_writer);
     CU_ASSERT_EQUAL_FATAL(ret, DDS_RETCODE_OK);
     g_writer = 0;
@@ -942,8 +948,10 @@ CU_Test(ddsc_listener, data_available_delete_writer, .init=init_triggering_test,
     CU_ASSERT_EQUAL_FATAL(cb_reader, g_reader);
 
     /* Deleting the writer must trigger DATA_AVAILABLE as well */
+    ddsrt_mutex_lock(&g_mutex);
     cb_called = 0;
     cb_reader = 0;
+    ddsrt_mutex_unlock(&g_mutex);
     ret = dds_delete (g_writer);
     CU_ASSERT_EQUAL_FATAL(ret, DDS_RETCODE_OK);
     g_writer = 0;
@@ -991,8 +999,10 @@ CU_Test(ddsc_listener, data_available_delete_writer_disposed, .init=init_trigger
     } while (ret > 0);
 
     /* Deleting the writer should not trigger DATA_AVAILABLE with all instances empty & disposed */
+    ddsrt_mutex_lock(&g_mutex);
     cb_called = 0;
     cb_reader = 0;
+    ddsrt_mutex_unlock(&g_mutex);
     ret = dds_delete (g_writer);
     CU_ASSERT_EQUAL_FATAL(ret, DDS_RETCODE_OK);
     g_writer = 0;
@@ -1174,7 +1184,9 @@ CU_Test(ddsc_listener, liveliness_changed, .init=init_triggering_test, .fini=fin
     CU_ASSERT_EQUAL_FATAL(liveliness_changed.last_publication_handle, writer_hdl);
 
     /* Reset the trigger flags. */
+    ddsrt_mutex_lock(&g_mutex);
     cb_called = 0;
+    ddsrt_mutex_unlock(&g_mutex);
 
     /* Change liveliness again by deleting the writer. */
     dds_delete(g_writer);

src/core/ddsc/tests/participant.c

@@ -47,7 +47,8 @@ CU_Test(ddsc_participant, create_with_no_conf_no_env)
   dds_domainid_t domain_id;
   dds_domainid_t valid_domain=3;
 
-  ddsrt_unsetenv(DDS_PROJECT_NAME_NOSPACE_CAPS"_URI");
+  status = ddsrt_unsetenv(DDS_PROJECT_NAME_NOSPACE_CAPS"_URI");
+  CU_ASSERT_EQUAL_FATAL(status, DDS_RETCODE_OK);
 
   //valid specific domain value
   participant2 = dds_create_participant (valid_domain, NULL, NULL);