Improvements on compute_key generated code (#544)

* Refs #23926. Refactor compute_key generated code.

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

* Refs #23926. Synchronize calls to compute_key.

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

* Refs #23926. Avoid unused params warning.

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

* Refs #23926. Use local variable for MD5 context.

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

* Refs #23926. Only generate key buffer when necessary.

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

* Refs #23926. Improve safety against allocation failures.

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

* Refs #23926. Bonus: some linter related improvements.

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

---------

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

Author: MiguelCompany

src/main/java/com/eprosima/fastcdr/idl/templates/TypesHeader.stg

@@ -114,11 +114,12 @@ $definition_list$
 annotation(ctx, annotation) ::= <<
 $if(annotation.enums || annotation.typeDefs || annotation.constDecls)$
 namespace $annotation.name$ {
-    $annotation.enums : { enum | $enum_type(ctx=ctx, parent=annotation, enum=enum)$}; separator="\n"$
 
-    $annotation.typeDefs : { typedef | $typedef_decl(ctx=ctx, parent=annotation, typedefs=typedef, typedefs_type="", declarator_type="")$}; separator="\n"$
+$annotation.enums : { enum | $enum_type(ctx=ctx, parent=annotation, enum=enum)$}; separator="\n"$
 
-    $annotation.constDecls : { const | $const_decl(ctx=ctx, parent=annotation, const=const, const_type="")$}; separator="\n"$
+$annotation.typeDefs : { typedef | $typedef_decl(ctx=ctx, parent=annotation, typedefs=typedef, typedefs_type="", declarator_type="")$}; separator="\n"$
+
+$annotation.constDecls : { const | $const_decl(ctx=ctx, parent=annotation, const=const, const_type="")$}; separator="\n"$
 
 } // namespace $annotation.name$
 $endif$

src/main/java/com/eprosima/fastdds/idl/templates/DDSPubSubTypeHeader.stg

@@ -23,6 +23,8 @@ $fileHeader(ctx=ctx,  file=[ctx.filename, "PubSubTypes.hpp"], description=["This
 #ifndef FAST_DDS_GENERATED__$ctx.headerGuardName$_PUBSUBTYPES_HPP
 #define FAST_DDS_GENERATED__$ctx.headerGuardName$_PUBSUBTYPES_HPP
 
+#include <mutex>
+
 #include <fastdds/dds/core/policy/QosPolicies.hpp>
 $if(ctx.thereIsInterface)$
 #include <fastdds/dds/rpc/ServiceTypeSupport.hpp>
@@ -48,10 +50,9 @@ $"\n"$
 >>
 
 module(ctx, parent, module, definition_list) ::= <<
-namespace $module.name$
-{
-    $definition_list$
-} // namespace $module.name$
+namespace $module.name$ {
+$definition_list$
+}  // namespace $module.name$
 >>
 
 typedef_decl(ctx, parent, typedefs, typedefs_type, declarator_type) ::= <<
@@ -188,8 +189,12 @@ public:
 
 private:
 
-    eprosima::fastdds::MD5 md5_;
-    unsigned char* key_buffer_;
+$if(struct.hasKey)$
+    unsigned char* key_buffer_ = nullptr;
+    std::mutex compute_key_mtx_;
+
+    unsigned char* get_key_buffer_nts();
+$endif$
 
 $if(struct.isPlain)$
 

src/main/java/com/eprosima/fastdds/idl/templates/DDSPubSubTypeSource.stg

@@ -21,6 +21,9 @@ $fileHeader(ctx=ctx,  file=[ctx.filename, "PubSubTypes.cpp"], description=["This
 
 #include "$ctx.filename$PubSubTypes.hpp"
 
+#include <algorithm>
+#include <mutex>
+
 #include <fastdds/dds/log/Log.hpp>
 #include <fastdds/rtps/common/CdrSerialization.hpp>
 
@@ -34,7 +37,6 @@ using InstanceHandle_t = eprosima::fastdds::rtps::InstanceHandle_t;
 using DataRepresentationId_t = eprosima::fastdds::dds::DataRepresentationId_t;
 
 $definitions; separator="\n"$
-
 $if(ctx.thereIsStructOrUnion || ctx.thereIsInterface)$
 // Include auxiliary functions like for serializing/deserializing.
 #include "$ctx.filename$CdrAux.ipp"
@@ -44,7 +46,7 @@ $endif$
 
 module(ctx, parent, module, definition_list) ::= <<
 namespace $module.name$ {
-    $definition_list$
+$definition_list$
 } // namespace $module.name$
 
 >>
@@ -58,26 +60,32 @@ $struct.name$PubSubType::$struct.name$PubSubType()
     uint32_t type_size = $struct.cScopedname$_max_cdr_typesize;
     type_size += static_cast<uint32_t>(eprosima::fastcdr::Cdr::alignment(type_size, 4)); /* possible submessage alignment */
     max_serialized_type_size = type_size + 4; /*encapsulation*/
-    is_compute_key_provided = $if(struct.hasKey)$true$else$false$endif$;
-    uint32_t key_length = $struct.cScopedname$_max_key_cdr_typesize > 16 ? $struct.cScopedname$_max_key_cdr_typesize : 16;
-    key_buffer_ = reinterpret_cast<unsigned char*>(malloc(key_length));
-    memset(key_buffer_, 0, key_length);
+$if(struct.hasKey)$
+    is_compute_key_provided = true;
+    key_buffer_ = nullptr;
+    get_key_buffer_nts();
+$else$
+    is_compute_key_provided = false;
+$endif$
 }
 
 $struct.name$PubSubType::~$struct.name$PubSubType()
 {
+$if(struct.hasKey)$
     if (key_buffer_ != nullptr)
     {
         free(key_buffer_);
     }
+$endif$
 }
 
 bool $struct.name$PubSubType::serialize(
         const void* const data,
         SerializedPayload_t& payload,
         DataRepresentationId_t data_representation)
 {
-    const ::$struct.scopedname$* p_type = static_cast<const ::$struct.scopedname$*>(data);
+    const ::$struct.scopedname$* p_type =
+            static_cast<const ::$struct.scopedname$*>(data);
 
     // Object that manages the raw buffer.
     eprosima::fastcdr::FastBuffer fastbuffer(reinterpret_cast<char*>(payload.data), payload.max_size);
@@ -97,7 +105,7 @@ bool $struct.name$PubSubType::serialize(
         ser.serialize_encapsulation();
         // Serialize the object.
         ser << *p_type;
-        ser.set_dds_cdr_options({0,0});
+        ser.set_dds_cdr_options({0, 0});
     }
     catch (eprosima::fastcdr::exception::Exception& /*exception*/)
     {
@@ -116,7 +124,8 @@ bool $struct.name$PubSubType::deserialize(
     try
     {
         // Convert DATA to pointer of your type
-        ::$struct.scopedname$* p_type = static_cast<::$struct.scopedname$*>(data);
+        ::$struct.scopedname$* p_type =
+                static_cast<::$struct.scopedname$*>(data);
 
         // Object that manages the raw buffer.
         eprosima::fastcdr::FastBuffer fastbuffer(reinterpret_cast<char*>(payload.data), payload.length);
@@ -149,9 +158,10 @@ uint32_t $struct.name$PubSubType::calculate_serialized_size(
             data_representation == DataRepresentationId_t::XCDR_DATA_REPRESENTATION ?
             eprosima::fastcdr::CdrVersion::XCDRv1 :eprosima::fastcdr::CdrVersion::XCDRv2);
         size_t current_alignment {0};
-        return static_cast<uint32_t>(calculator.calculate_serialized_size(
-                    *static_cast<const ::$struct.scopedname$*>(data), current_alignment)) +
-                4u /*encapsulation*/;
+        const ::$struct.scopedname$* p_type =
+                static_cast<const ::$struct.scopedname$*>(data);
+        auto calc_size = calculator.calculate_serialized_size(*p_type, current_alignment);
+        return static_cast<uint32_t>(calc_size) + 4u /*encapsulation*/;
     }
     catch (eprosima::fastcdr::exception::Exception& /*exception*/)
     {
@@ -175,16 +185,17 @@ bool $struct.name$PubSubType::compute_key(
         InstanceHandle_t& handle,
         bool force_md5)
 {
-    if (!is_compute_key_provided)
-    {
-        return false;
-    }
-
+$if(struct.hasKey)$
     ::$struct.scopedname$ data;
     if (deserialize(payload, static_cast<void*>(&data)))
     {
         return compute_key(static_cast<void*>(&data), handle, force_md5);
     }
+$else$
+    static_cast<void>(payload);
+    static_cast<void>(handle);
+    static_cast<void>(force_md5);
+$endif$
 
     return false;
 }
@@ -194,48 +205,80 @@ bool $struct.name$PubSubType::compute_key(
         InstanceHandle_t& handle,
         bool force_md5)
 {
-    if (!is_compute_key_provided)
+$if(struct.hasKey)$
+    std::lock_guard<std::mutex> guard(compute_key_mtx_);
+    const ::$struct.scopedname$* p_type =
+            static_cast<const ::$struct.scopedname$*>(data);
+
+    // Ensure the key buffer is reserved
+    unsigned char* key_buffer = get_key_buffer_nts();
+    if (key_buffer == nullptr)
     {
         return false;
     }
 
-    const ::$struct.scopedname$* p_type = static_cast<const ::$struct.scopedname$*>(data);
-
     // Object that manages the raw buffer.
-    eprosima::fastcdr::FastBuffer fastbuffer(reinterpret_cast<char*>(key_buffer_),
+    eprosima::fastcdr::FastBuffer fastbuffer(reinterpret_cast<char*>(key_buffer),
             $struct.cScopedname$_max_key_cdr_typesize);
 
     // Object that serializes the data.
-    eprosima::fastcdr::Cdr ser(fastbuffer, eprosima::fastcdr::Cdr::BIG_ENDIANNESS, eprosima::fastcdr::CdrVersion::XCDRv2);
+    eprosima::fastcdr::Cdr ser(
+        fastbuffer,
+        eprosima::fastcdr::Cdr::BIG_ENDIANNESS,
+        eprosima::fastcdr::CdrVersion::XCDRv2);
     ser.set_encoding_flag(eprosima::fastcdr::EncodingAlgorithmFlag::PLAIN_CDR2);
     eprosima::fastcdr::serialize_key(ser, *p_type);
     if (force_md5 || $struct.cScopedname$_max_key_cdr_typesize > 16)
     {
-        md5_.init();
-        md5_.update(key_buffer_, static_cast<unsigned int>(ser.get_serialized_data_length()));
-        md5_.finalize();
+        eprosima::fastdds::MD5 md5;
+        md5.init();
+        md5.update(key_buffer, static_cast<unsigned int>(ser.get_serialized_data_length()));
+        md5.finalize();
         for (uint8_t i = 0; i < 16; ++i)
         {
-            handle.value[i] = md5_.digest[i];
+            handle.value[i] = md5.digest[i];
         }
     }
     else
     {
         for (uint8_t i = 0; i < 16; ++i)
         {
-            handle.value[i] = key_buffer_[i];
+            handle.value[i] = key_buffer[i];
         }
     }
     return true;
+$else$
+    static_cast<void>(data);
+    static_cast<void>(handle);
+    static_cast<void>(force_md5);
+
+    return false;
+$endif$
 }
 
+$if(struct.hasKey)$
+unsigned char* $struct.name$PubSubType::get_key_buffer_nts()
+{
+    // If already reserved, return
+    if (key_buffer_ != nullptr)
+    {
+        return key_buffer_;
+    }
+
+    // Allocate the key buffer
+    uint32_t key_length = (std::max)($struct.cScopedname$_max_key_cdr_typesize, 16u);
+    key_buffer_ = reinterpret_cast<unsigned char*>(calloc(key_length, 1u));
+    return key_buffer_;
+}
+$endif$
+
 void $struct.name$PubSubType::register_type_object_representation()
 {
     $if (ctx.generateTypeObjectSupport)$
     register_$struct.name$_type_identifier(type_identifiers_);
     $else$
     EPROSIMA_LOG_WARNING(XTYPES_TYPE_REPRESENTATION,
-        "TypeObject type representation support disabled in generated code");
+            "TypeObject type representation support disabled in generated code");
     $endif$
 }
 $endif$
@@ -310,7 +353,7 @@ public:
             ser.serialize_encapsulation();
             // Serialize the object.
             ser << *p_type;
-            ser.set_dds_cdr_options({0,0});
+            ser.set_dds_cdr_options({0, 0});
         }
         catch (eprosima::fastcdr::exception::Exception& /*exception*/)
         {
@@ -362,9 +405,9 @@ public:
                 data_representation == DataRepresentationId_t::XCDR_DATA_REPRESENTATION ?
                 eprosima::fastcdr::CdrVersion::XCDRv1 : eprosima::fastcdr::CdrVersion::XCDRv2);
             size_t current_alignment {0};
-            return static_cast<uint32_t>(calculator.calculate_serialized_size(
-                        *static_cast<const type*>(data), current_alignment)) +
-                    4u /*encapsulation*/;
+            const type* p_type = static_cast<const type*>(data);
+            auto calc_size = calculator.calculate_serialized_size(*p_type, current_alignment);
+            return static_cast<uint32_t>(calc_size) + 4u /*encapsulation*/;
         }
         catch (eprosima::fastcdr::exception::Exception& /*exception*/)
         {
@@ -412,7 +455,7 @@ $if (ctx.generateTypeObjectSupport)$
         register_$interface.name$_$suffix$_type_identifier(type_identifiers_);
 $else$
         EPROSIMA_LOG_WARNING(XTYPES_TYPE_REPRESENTATION,
-            "TypeObject type representation support disabled in generated code");
+                "TypeObject type representation support disabled in generated code");
 $endif$
     }
 

src/main/java/com/eprosima/fastdds/idl/templates/InterfaceDetails.stg

@@ -43,7 +43,7 @@ bitset_type(ctx, parent, bitset) ::= <<>>
 
 module(ctx, parent, module, definition_list) ::= <<
 namespace $module.name$ {
-    $definition_list$
+$definition_list$
 }  // namespace $module.name$
 >>
 

src/main/java/com/eprosima/fastdds/idl/templates/SerializationHeader.stg

@@ -101,6 +101,6 @@ $endif$
 
 module(ctx, parent, module, definition_list) ::= <<
 namespace $module.name$ {
-    $definition_list$
+$definition_list$
 }  // namespace $module.name$
 >>

src/main/java/com/eprosima/fastdds/idl/templates/XTypesTypeObjectHeader.stg

@@ -60,11 +60,12 @@ $definitions; separator="\n"$
 annotation(ctx, annotation) ::= <<
 
 namespace $annotation.name$ {
-    $annotation.enums : { enum | $enum_type(ctx=ctx, parent=annotation, enum=enum)$}; separator="\n"$
 
-    $annotation.typeDefs : { typedef | $typedef_decl(ctx=ctx, parent=annotation, typedefs=typedef, typedefs_type="", declarator_type="")$}; separator="\n"$
+$annotation.enums : { enum | $enum_type(ctx=ctx, parent=annotation, enum=enum)$}; separator="\n"$
 
-    $annotation.constDecls : { const | $const_decl(ctx=ctx, parent=annotation, const=const, const_type="")$}; separator="\n"$
+$annotation.typeDefs : { typedef | $typedef_decl(ctx=ctx, parent=annotation, typedefs=typedef, typedefs_type="", declarator_type="")$}; separator="\n"$
+
+$annotation.constDecls : { const | $const_decl(ctx=ctx, parent=annotation, const=const, const_type="")$}; separator="\n"$
 
 } // namespace $annotation.name$
 

src/main/java/com/eprosima/fastdds/idl/templates/XTypesTypeObjectSource.stg

@@ -60,11 +60,12 @@ $definitions; separator=""$
 annotation(ctx, annotation) ::= <<
 
 namespace $annotation.name$ {
-    $annotation.enums : { enum | $enum_type(ctx=ctx, parent=annotation, enum=enum)$}; separator="\n"$
 
-    $annotation.typeDefs : { typedef | $typedef_decl(ctx=ctx, parent=annotation, typedefs=typedef, typedefs_type="", declarator_type="")$}; separator="\n"$
+$annotation.enums : { enum | $enum_type(ctx=ctx, parent=annotation, enum=enum)$}; separator="\n"$
 
-    $annotation.constDecls : { const | $const_decl(ctx=ctx, parent=annotation, const=const, const_type="")$}; separator="\n"$
+$annotation.typeDefs : { typedef | $typedef_decl(ctx=ctx, parent=annotation, typedefs=typedef, typedefs_type="", declarator_type="")$}; separator="\n"$
+
+$annotation.constDecls : { const | $const_decl(ctx=ctx, parent=annotation, const=const, const_type="")$}; separator="\n"$
 
 } // namespace $annotation.name$