Addressed comments from PR ros2#123

Signed-off-by: Sid Faber <[email protected]>

README.md

@@ -27,8 +27,3 @@ If you want to use a pre-existing installation of Cyclone DDS, you don't need to
 may have to tell CMake where to look for it using the `CycloneDDS_DIR` variable.  That also appears
 to be the case if there are other packages in the ROS2 workspace that you would like to use Cyclone
 DDS directly instead of via the ROS2 abstraction.
-
-## Known limitations
-
-Cyclone DDS doesn't yet fully implement the Lifespan, Deadline and some of the Liveliness QoS modes.
-Consequently these features of ROS2 are also not yet supported when using Cyclone DDS.

rmw_cyclonedds_cpp/src/rmw_node.cpp

@@ -27,10 +27,10 @@
 #include <regex>
 
 #include "rcutils/filesystem.h"
+#include "rcutils/format_string.h"
 #include "rcutils/get_env.h"
 #include "rcutils/logging_macros.h"
 #include "rcutils/strdup.h"
-#include "rcutils/format_string.h"
 
 #include "rmw/allocators.h"
 #include "rmw/convert_rcutils_ret_to_rmw_ret.h"
@@ -81,6 +81,13 @@
 #define SUPPORT_LOCALHOST 0
 #endif
 
+/* QOS Property List support exists in Cyclone if and only if security features are available */
+#ifdef DDS_HAS_QOS_PROPERTY_LIST
+#define SUPPORT_SECURITY 1
+#else
+#define SUPPORT_SECURITY 0
+#endif
+
 /* Set to > 0 for printing warnings to stderr for each messages that was taken more than this many
    ms after writing */
 #define REPORT_LATE_MESSAGES 0
@@ -644,6 +651,9 @@ static std::string get_node_user_data(const char * node_name, const char * node_
          std::string(";");
 }
 
+/* QOS propery list features exit in cyclone IFF security is supported */
+#if SUPPORT_SECURITY
+
 /*  Returns the full URI of a security file properly formatted for DDS  */
 char * get_security_file_URI(
   const char * security_filename, const char * node_secure_root,
@@ -682,7 +692,7 @@ void store_security_filepath_in_qos(
 }
 
 /*  Set all the qos properties needed to enable DDS security  */
-void configure_qos_for_security(
+rmw_ret_t configure_qos_for_security(
   dds_qos_t * qos, const rmw_node_security_options_t * security_options)
 {
   /*  File path is set to nullptr if file does not exist or is not readable  */
@@ -705,18 +715,32 @@ void configure_qos_for_security(
     qos, "dds.sec.access.permissions", "permissions.p7s",
     security_options);
 
-  dds_qset_prop(qos, "dds.sec.auth.library.path", "libdds_security_auth.so");
+  dds_qset_prop(qos, "dds.sec.auth.library.path", "dds_security_auth");
   dds_qset_prop(qos, "dds.sec.auth.library.init", "init_authentication");
   dds_qset_prop(qos, "dds.sec.auth.library.finalize", "finalize_authentication");
 
-  dds_qset_prop(qos, "dds.sec.crypto.library.path", "libdds_security_crypto.so");
+  dds_qset_prop(qos, "dds.sec.crypto.library.path", "dds_security_crypto");
   dds_qset_prop(qos, "dds.sec.crypto.library.init", "init_crypto");
   dds_qset_prop(qos, "dds.sec.crypto.library.finalize", "finalize_crypto");
 
-  dds_qset_prop(qos, "dds.sec.access.library.path", "libdds_security_ac.so");
+  dds_qset_prop(qos, "dds.sec.access.library.path", "dds_security_ac");
   dds_qset_prop(qos, "dds.sec.access.library.init", "init_access_control");
   dds_qset_prop(qos, "dds.sec.access.library.finalize", "finalize_access_control");
+
+  return RMW_RET_OK;
 }
+#else
+/* Fail when security is requested but not available */
+rcutils_ret_t configure_qos_for_security(
+  dds_qos_t * qos, const rmw_node_security_options_t * security_options)
+{
+  (void) qos;
+  (void) security_options;
+  RMW_SET_ERROR_MSG(
+    "Security was requested but this Cyclone version does not have security support enabled.");
+    return RMW_RET_UNSUPPORTED;
+}
+#endif
 
 extern "C" rmw_node_t * rmw_create_node(
   rmw_context_t * context, const char * name,
@@ -743,11 +767,7 @@ extern "C" rmw_node_t * rmw_create_node(
   const dds_domainid_t did = DDS_DOMAIN_DEFAULT;
 #endif
 
-  if (security_options == nullptr) {
-    RCUTILS_LOG_ERROR_NAMED(
-      "rmw_cyclonedds_cpp", "rmw_create_node: security options null");
-    return nullptr;
-  }
+  RCUTILS_CHECK_ARGUMENT_FOR_NULL(security_options, nullptr);
 
   rmw_ret_t ret;
   int dummy_validation_result;
@@ -770,16 +790,15 @@ extern "C" rmw_node_t * rmw_create_node(
 #endif
 
   dds_qos_t * qos = dds_create_qos();
-  if (qos == nullptr) {
-    RCUTILS_LOG_ERROR_NAMED(
-      "rmw_cyclonedds_cpp", "rmw_create_node: Unable to create qos");
-    return nullptr;
-  }
+  RCUTILS_CHECK_FOR_NULL_WITH_MSG(
+    security_options, "rmw_create_node: Unable to create qos", return nullptr);
   std::string user_data = get_node_user_data(name, namespace_);
   dds_qset_userdata(qos, user_data.c_str(), user_data.size());
 
   if (security_options->enforce_security) {
-    configure_qos_for_security(qos, security_options);
+    if (configure_qos_for_security(qos, security_options) != RMW_RET_OK) {
+      return nullptr;
+    };
   }
 
   dds_entity_t pp = dds_create_participant(did, qos, nullptr);