From 9c8f0097ebaa5d4687799ef2383c2f17c9115439 Mon Sep 17 00:00:00 2001 From: Joachim Achtzehnter Date: Fri, 11 Sep 2020 18:10:17 -0700 Subject: [PATCH 1/4] Fix server-side fragmentation Old implementation attempted to implement fragmentation in the TAO_OutputCDR inserters, but this does not work because array and sequence types, including strings, were implemented at the ACE_OutputCDR layer, which remained oblivious to fragmentation. The old implementation also left TAO_OutputCDR with a dangerous interface. The write_xxx function inherited from ACE_OutputCDR did not fragment, only the inserters did. The new implementation fragments in the low-level write_1, write_2, write_4, write_8, write_16, and write_array functions, making these virtual to ensure that constructed types like arrays and sequences correctly fragment their elements, and similar for structs, etc. --- ACE/ace/CDR_Stream.h | 44 +++-- TAO/tao/CDR.cpp | 173 +++++++++++++++++++ TAO/tao/CDR.h | 13 ++ TAO/tao/CDR.inl | 64 ++----- TAO/tao/GIOP_Fragmentation_Strategy.h | 3 + TAO/tao/Null_Fragmentation_Strategy.cpp | 6 + TAO/tao/Null_Fragmentation_Strategy.h | 2 +- TAO/tao/On_Demand_Fragmentation_Strategy.cpp | 41 +++-- TAO/tao/On_Demand_Fragmentation_Strategy.h | 3 + 9 files changed, 264 insertions(+), 85 deletions(-) diff --git a/ACE/ace/CDR_Stream.h b/ACE/ace/CDR_Stream.h index 4ce2402a5fb4e..0c6fa775af81f 100644 --- a/ACE/ace/CDR_Stream.h +++ b/ACE/ace/CDR_Stream.h @@ -165,7 +165,7 @@ class ACE_Export ACE_OutputCDR ACE_CDR::Octet giop_minor_version = ACE_CDR_GIOP_MINOR_VERSION); /// destructor - ~ACE_OutputCDR (void); + virtual ~ACE_OutputCDR (void); /** * Disambiguate overload when inserting booleans, octets, chars, and @@ -523,20 +523,14 @@ class ACE_Export ACE_OutputCDR void unregister_monitor (void); #endif /* ACE_HAS_MONITOR_POINTS==1 */ -private: - // Find the message block in the chain of message blocks - // that the provide location locates. - ACE_Message_Block* find (char* loc); - - /// disallow copying... - ACE_OutputCDR (const ACE_OutputCDR& rhs); - ACE_OutputCDR& operator= (const ACE_OutputCDR& rhs); +protected: - ACE_CDR::Boolean write_1 (const ACE_CDR::Octet *x); - ACE_CDR::Boolean write_2 (const ACE_CDR::UShort *x); - ACE_CDR::Boolean write_4 (const ACE_CDR::ULong *x); - ACE_CDR::Boolean write_8 (const ACE_CDR::ULongLong *x); - ACE_CDR::Boolean write_16 (const ACE_CDR::LongDouble *x); + // These are virtual to support GIOP fragmentation at the TAO layer. + virtual ACE_CDR::Boolean write_1 (const ACE_CDR::Octet *x); + virtual ACE_CDR::Boolean write_2 (const ACE_CDR::UShort *x); + virtual ACE_CDR::Boolean write_4 (const ACE_CDR::ULong *x); + virtual ACE_CDR::Boolean write_8 (const ACE_CDR::ULongLong *x); + virtual ACE_CDR::Boolean write_16 (const ACE_CDR::LongDouble *x); /** * write an array of @a length elements, each of @a size bytes and the @@ -549,12 +543,26 @@ class ACE_Export ACE_OutputCDR * but for several elements @c memcpy should be more efficient, it * could be interesting to find the break even point and optimize * for that case, but that would be too platform dependent. + * + * This is virtual to support GIOP fragmentation at the TAO layer. */ - ACE_CDR::Boolean write_array (const void *x, - size_t size, - size_t align, - ACE_CDR::ULong length); + virtual ACE_CDR::Boolean write_array (const void *x, + size_t size, + size_t align, + ACE_CDR::ULong length); + + // Overrides of the above functions may need to set this bit. + void good_bit (bool bit) { good_bit_ = bit; } + +private: + // Find the message block in the chain of message blocks + // that the provide location locates. + ACE_Message_Block* find (char* loc); + + /// disallow copying... + ACE_OutputCDR (const ACE_OutputCDR& rhs); + ACE_OutputCDR& operator= (const ACE_OutputCDR& rhs); ACE_CDR::Boolean write_wchar_array_i (const ACE_CDR::WChar* x, ACE_CDR::ULong length); diff --git a/TAO/tao/CDR.cpp b/TAO/tao/CDR.cpp index 53c4318b9c135..cb1108511a5b3 100644 --- a/TAO/tao/CDR.cpp +++ b/TAO/tao/CDR.cpp @@ -244,7 +244,17 @@ TAO_OutputCDR::fragment_stream (ACE_CDR::ULong pending_alignment, return true; // Success. } +ACE_CDR::ULong +TAO_OutputCDR::fragment_bytes_available (ACE_CDR::ULong pending_alignment) +{ + if (this->fragmentation_strategy_) + { + return this->fragmentation_strategy_->available (*this, + pending_alignment); + } + return 0xFFFFFFFF; +} int TAO_OutputCDR::offset (char* pos) @@ -280,6 +290,169 @@ TAO_OutputCDR::offset (char* pos) return offset; } +ACE_CDR::Boolean +TAO_OutputCDR::write_1 (const ACE_CDR::Octet *x) +{ + return + fragment_stream (ACE_CDR::OCTET_ALIGN, sizeof (CORBA::Octet)) + && ACE_OutputCDR::write_1 (x); +} + +ACE_CDR::Boolean +TAO_OutputCDR::write_2 (const ACE_CDR::UShort *x) +{ + return + fragment_stream (ACE_CDR::SHORT_ALIGN, sizeof (CORBA::UShort)) + && ACE_OutputCDR::write_2 (x); +} + +ACE_CDR::Boolean +TAO_OutputCDR::write_4 (const ACE_CDR::ULong *x) +{ + return + fragment_stream (ACE_CDR::LONG_ALIGN, sizeof (CORBA::ULong)) + && ACE_OutputCDR::write_4 (x); +} + +ACE_CDR::Boolean +TAO_OutputCDR::write_8 (const ACE_CDR::ULongLong *x) +{ + return + fragment_stream (ACE_CDR::LONGLONG_ALIGN, sizeof (CORBA::ULongLong)) + && ACE_OutputCDR::write_8 (x); +} + +ACE_CDR::Boolean +TAO_OutputCDR::write_16 (const ACE_CDR::LongDouble *x) +{ + if (!fragment_stream (ACE_CDR::LONGLONG_ALIGN, sizeof (CORBA::ULongLong))) + return 0; + + ACE_CDR::ULong avail = fragment_bytes_available(ACE_CDR::LONGLONG_ALIGN); + + if (avail < sizeof (CORBA::LongDouble)) + { + const CORBA::ULongLong* ptr_8 + = reinterpret_cast (x); + +#if !defined (ACE_ENABLE_SWAP_ON_WRITE) + + return + ACE_OutputCDR::write_8 (ptr_8) + && fragment_stream (ACE_CDR::LONGLONG_ALIGN, sizeof (CORBA::ULongLong)) + && ACE_OutputCDR::write_8 (ptr_8 + 1); + +#else + + if (!this->do_byte_swap_) + { + return + ACE_OutputCDR::write_8 (ptr_8) + && fragment_stream (ACE_CDR::LONGLONG_ALIGN, sizeof (CORBA::ULongLong)) + && ACE_OutputCDR::write_8 (ptr_8 + 1); + } + else + { + return + ACE_OutputCDR::write_8 (ptr_8 + 1) + && fragment_stream (ACE_CDR::LONGLONG_ALIGN, sizeof (CORBA::ULongLong)) + && ACE_OutputCDR::write_8 (ptr_8); + } + +#endif + + } + else + { + return ACE_OutputCDR::write_16 (x); + } +} + +ACE_CDR::Boolean +TAO_OutputCDR::write_array (const void *x, + size_t size, + size_t align, + ACE_CDR::ULong length) +{ + if (length == 0) + return 1; + + const char* xPtr = static_cast (x); + + if (size > ACE_CDR::MAX_ALIGNMENT) + { + if (size != 16) + { + good_bit(false); + return 0; + } + + // may need to fragment in the middle of an element + + if (!fragment_stream (align, ACE_CDR::MAX_ALIGNMENT)) + return 0; + + while (true) + { + ACE_CDR::ULong availableBytes = fragment_bytes_available(align); + ACE_CDR::ULong availableLength = availableBytes / size; + ACE_CDR::ULong batchLength; + bool lastBatch; + + if (availableLength == 0) + { + // This will fragment in the middle of the 16-byte element. + if (!write_16 (reinterpret_cast (xPtr))) + return 0; + batchLength = 1; + lastBatch = (length == 1); + } + else + { + // We can write a batch of whole elements into the current fragment. + lastBatch = (availableLength >= length); + batchLength = (lastBatch ? length : availableLength); + + if (!ACE_OutputCDR::write_array (xPtr, size, align, batchLength)) + return 0; + } + + if (lastBatch) + return 1; + + if (!fragment_stream (align, ACE_CDR::MAX_ALIGNMENT)) + return 0; + + xPtr += batchLength * size; + length -= batchLength; + } + } + else + { + if (!fragment_stream (align, size)) + return 0; + + while (true) + { + ACE_CDR::ULong availableBytes = fragment_bytes_available(align); + ACE_CDR::ULong availableLength = availableBytes / size; + bool lastBatch = (availableLength >= length); + ACE_CDR::ULong batchLength = (lastBatch ? length : availableLength); + + if (!ACE_OutputCDR::write_array (xPtr, size, align, batchLength)) + return 0; + + if (lastBatch) + return 1; + + if (!fragment_stream (align, size)) + return 0; + + xPtr += batchLength * size; + length -= batchLength; + } + } +} // **************************************************************** diff --git a/TAO/tao/CDR.h b/TAO/tao/CDR.h index f7406c6000d49..7b79e3ec390c6 100644 --- a/TAO/tao/CDR.h +++ b/TAO/tao/CDR.h @@ -188,6 +188,7 @@ class TAO_Export TAO_OutputCDR : public ACE_OutputCDR */ bool fragment_stream (ACE_CDR::ULong pending_alignment, ACE_CDR::ULong pending_length); + ACE_CDR::ULong fragment_bytes_available(ACE_CDR::ULong pending_alignment); /// Are there more data fragments to come? bool more_fragments (void) const; @@ -239,6 +240,18 @@ class TAO_Export TAO_OutputCDR : public ACE_OutputCDR /// Calculate the offset between pos and current wr_ptr. int offset (char* pos); +protected: + // These are overridden to support GIOP fragmentation. + virtual ACE_CDR::Boolean write_1 (const ACE_CDR::Octet *x); + virtual ACE_CDR::Boolean write_2 (const ACE_CDR::UShort *x); + virtual ACE_CDR::Boolean write_4 (const ACE_CDR::ULong *x); + virtual ACE_CDR::Boolean write_8 (const ACE_CDR::ULongLong *x); + virtual ACE_CDR::Boolean write_16 (const ACE_CDR::LongDouble *x); + + virtual ACE_CDR::Boolean write_array (const void *x, + size_t size, + size_t align, + ACE_CDR::ULong length); private: // disallow copying... TAO_OutputCDR (const TAO_OutputCDR& rhs); diff --git a/TAO/tao/CDR.inl b/TAO/tao/CDR.inl index c4fb156fcc5dc..78464cce62137 100644 --- a/TAO/tao/CDR.inl +++ b/TAO/tao/CDR.inl @@ -331,28 +331,19 @@ TAO_InputCDR::reset_vt_indirect_maps () ACE_INLINE CORBA::Boolean operator<< (TAO_OutputCDR &os, CORBA::Short x) { - return - os.fragment_stream (ACE_CDR::SHORT_ALIGN, - sizeof (CORBA::Short)) - && static_cast (os) << x; + return static_cast (os) << x; } ACE_INLINE CORBA::Boolean operator<< (TAO_OutputCDR &os, CORBA::UShort x) { - return - os.fragment_stream (ACE_CDR::SHORT_ALIGN, - sizeof (CORBA::UShort)) - && static_cast (os) << x; + return static_cast (os) << x; } ACE_INLINE CORBA::Boolean operator<< (TAO_OutputCDR &os, CORBA::Long x) { - return - os.fragment_stream (ACE_CDR::LONG_ALIGN, - sizeof (CORBA::Long)) - && static_cast (os) << x; + return static_cast (os) << x; } ACE_INLINE CORBA::Boolean operator<< (TAO_OutputCDR &os, @@ -367,66 +358,43 @@ ACE_INLINE CORBA::Boolean operator<< (TAO_OutputCDR &os, ACE_INLINE CORBA::Boolean operator<< (TAO_OutputCDR &os, CORBA::LongLong x) { - return - os.fragment_stream (ACE_CDR::LONGLONG_ALIGN, - sizeof (CORBA::LongLong)) - && static_cast (os) << x; + return static_cast (os) << x; } ACE_INLINE CORBA::Boolean operator<< (TAO_OutputCDR &os, CORBA::ULongLong x) { - return - os.fragment_stream (ACE_CDR::LONGLONG_ALIGN, - sizeof (CORBA::ULongLong)) - && static_cast (os) << x; + return static_cast (os) << x; } ACE_INLINE CORBA::Boolean operator<< (TAO_OutputCDR& os, CORBA::LongDouble x) { - return - os.fragment_stream (ACE_CDR::LONGDOUBLE_ALIGN, - sizeof (CORBA::LongDouble)) - && static_cast (os) << x; + return static_cast (os) << x; } ACE_INLINE CORBA::Boolean operator<< (TAO_OutputCDR &os, CORBA::Float x) { - return - os.fragment_stream (ACE_CDR::LONG_ALIGN, - sizeof (CORBA::Float)) - && static_cast (os) << x; + return static_cast (os) << x; } ACE_INLINE CORBA::Boolean operator<< (TAO_OutputCDR &os, CORBA::Double x) { - return - os.fragment_stream (ACE_CDR::LONGLONG_ALIGN, - sizeof (CORBA::Double)) - && static_cast (os) << x; + return static_cast (os) << x; } ACE_INLINE CORBA::Boolean operator<< (TAO_OutputCDR &os, const char * x) { - return - os.fragment_stream (ACE_CDR::OCTET_ALIGN, - sizeof (char)) - && static_cast (os) << x; + return static_cast (os) << x; } ACE_INLINE CORBA::Boolean operator<< (TAO_OutputCDR &os, const CORBA::WChar * x) { - return - os.fragment_stream ((sizeof (CORBA::WChar) == 2 - ? ACE_CDR::SHORT_ALIGN - : ACE_CDR::LONG_ALIGN), - sizeof (CORBA::WChar)) - && static_cast (os) << x; + return static_cast (os) << x; } ACE_INLINE CORBA::Boolean operator<< (TAO_OutputCDR &os, @@ -455,10 +423,7 @@ ACE_INLINE CORBA::Boolean operator<< (TAO_OutputCDR &os, const std::string &x) { #if defined (ACE_HAS_CPP11) - return - os.fragment_stream (ACE_CDR::OCTET_ALIGN, - sizeof (char)) - && static_cast (os) << x; + return static_cast (os) << x; #else return os << x.c_str (); #endif @@ -480,12 +445,7 @@ ACE_INLINE CORBA::Boolean operator<< (TAO_OutputCDR &os, const std::wstring &x) { #if defined (ACE_HAS_CPP11) - return - os.fragment_stream ((sizeof (CORBA::WChar) == 2 - ? ACE_CDR::SHORT_ALIGN - : ACE_CDR::LONG_ALIGN), - sizeof (CORBA::WChar)) - && static_cast (os) << x; + return static_cast (os) << x; #else return os << x.c_str (); #endif diff --git a/TAO/tao/GIOP_Fragmentation_Strategy.h b/TAO/tao/GIOP_Fragmentation_Strategy.h index be6d1f4303463..5679e909d2dfc 100644 --- a/TAO/tao/GIOP_Fragmentation_Strategy.h +++ b/TAO/tao/GIOP_Fragmentation_Strategy.h @@ -67,6 +67,9 @@ class TAO_Export TAO_GIOP_Fragmentation_Strategy ACE_CDR::ULong pending_alignment, ACE_CDR::ULong pending_length) = 0; + virtual ACE_CDR::ULong available (TAO_OutputCDR & cdr, + ACE_CDR::ULong pending_alignment) = 0; + private: // Disallow copying and assignment. TAO_GIOP_Fragmentation_Strategy (TAO_GIOP_Fragmentation_Strategy const &); diff --git a/TAO/tao/Null_Fragmentation_Strategy.cpp b/TAO/tao/Null_Fragmentation_Strategy.cpp index d41d7f42d343d..c1e41194d5376 100644 --- a/TAO/tao/Null_Fragmentation_Strategy.cpp +++ b/TAO/tao/Null_Fragmentation_Strategy.cpp @@ -11,3 +11,9 @@ TAO_Null_Fragmentation_Strategy::fragment (TAO_OutputCDR &, { return 0; } + +ACE_CDR::ULong +TAO_Null_Fragmentation_Strategy::available (TAO_OutputCDR &, ACE_CDR::ULong) +{ + return 0xFFFFFFFF; +} diff --git a/TAO/tao/Null_Fragmentation_Strategy.h b/TAO/tao/Null_Fragmentation_Strategy.h index 528b1a2871498..4fe49dc0cb41b 100644 --- a/TAO/tao/Null_Fragmentation_Strategy.h +++ b/TAO/tao/Null_Fragmentation_Strategy.h @@ -44,7 +44,7 @@ class TAO_Null_Fragmentation_Strategy TAO_Null_Fragmentation_Strategy (void) {} virtual ~TAO_Null_Fragmentation_Strategy (void); virtual int fragment (TAO_OutputCDR &, ACE_CDR::ULong, ACE_CDR::ULong); - + virtual ACE_CDR::ULong available (TAO_OutputCDR &, ACE_CDR::ULong); private: // Disallow copying and assignment. diff --git a/TAO/tao/On_Demand_Fragmentation_Strategy.cpp b/TAO/tao/On_Demand_Fragmentation_Strategy.cpp index e9b790dd6faee..5d39da6619c9a 100644 --- a/TAO/tao/On_Demand_Fragmentation_Strategy.cpp +++ b/TAO/tao/On_Demand_Fragmentation_Strategy.cpp @@ -12,8 +12,17 @@ TAO_On_Demand_Fragmentation_Strategy::TAO_On_Demand_Fragmentation_Strategy ( TAO_Transport * transport, CORBA::ULong max_message_size) : transport_ (transport) - , max_message_size_ (max_message_size) + , max_message_size_ ((max_message_size < 24 + ? 24 : max_message_size) + & ~(ACE_CDR::MAX_ALIGNMENT - 1)) { + // this->max_message_size_ must be >= 24 bytes, i.e.: + // 12 for GIOP protocol header + // + 4 for GIOP fragment header + // + 8 for payload (including padding) + // since fragments must be aligned on an 8 byte boundary. + // Make it a multiple of 8 to avoid checking for this repeatedly + // at runtime. } TAO_On_Demand_Fragmentation_Strategy::~TAO_On_Demand_Fragmentation_Strategy ( @@ -48,19 +57,7 @@ TAO_On_Demand_Fragmentation_Strategy::fragment ( ACE_align_binary (cdr.total_length (), pending_alignment) + pending_length); - // Except for the last fragment, fragmented GIOP messages must - // always be aligned on an 8-byte boundary. Padding will be added - // if necessary. - ACE_CDR::ULong const aligned_length = - ACE_Utils::truncate_cast ( - ACE_align_binary (total_pending_length, ACE_CDR::MAX_ALIGNMENT)); - - // this->max_message_size_ must be >= 24 bytes, i.e.: - // 12 for GIOP protocol header - // + 4 for GIOP fragment header - // + 8 for payload (including padding) - // since fragments must be aligned on an 8 byte boundary. - if (aligned_length > this->max_message_size_) + if (total_pending_length > this->max_message_size_) { // Pad the outgoing fragment if necessary. if (cdr.align_write_ptr (ACE_CDR::MAX_ALIGNMENT) != 0) @@ -93,3 +90,19 @@ TAO_On_Demand_Fragmentation_Strategy::fragment ( return 0; } + +ACE_CDR::ULong +TAO_On_Demand_Fragmentation_Strategy::available ( + TAO_OutputCDR & cdr, + ACE_CDR::ULong pending_alignment) +{ + // Determine increase in CDR stream length if pending data is + // marshaled, taking into account the alignment for the given data + // type. + ACE_CDR::ULong const total_starting_length = + ACE_Utils::truncate_cast ( + ACE_align_binary (cdr.total_length (), pending_alignment)); + + return (total_starting_length > this->max_message_size_ + ? 0 : this->max_message_size_ - total_starting_length); +} diff --git a/TAO/tao/On_Demand_Fragmentation_Strategy.h b/TAO/tao/On_Demand_Fragmentation_Strategy.h index ef893a8849a2f..4914b86ccdb1d 100644 --- a/TAO/tao/On_Demand_Fragmentation_Strategy.h +++ b/TAO/tao/On_Demand_Fragmentation_Strategy.h @@ -51,6 +51,9 @@ class TAO_On_Demand_Fragmentation_Strategy ACE_CDR::ULong pending_alignment, ACE_CDR::ULong pending_length); + virtual ACE_CDR::ULong available (TAO_OutputCDR & cdr, + ACE_CDR::ULong pending_alignment); + private: // Disallow copying and assignment. TAO_On_Demand_Fragmentation_Strategy (TAO_On_Demand_Fragmentation_Strategy const &); From a381ad7aace9ac68a1907c32df4dd0a87fc96387 Mon Sep 17 00:00:00 2001 From: Joachim Achtzehnter Date: Fri, 11 Sep 2020 19:09:31 -0700 Subject: [PATCH 2/4] Removed trailing white space. --- ACE/ace/CDR_Stream.h | 2 +- TAO/tao/CDR.cpp | 12 ++++++------ TAO/tao/On_Demand_Fragmentation_Strategy.cpp | 2 +- TAO/tao/On_Demand_Fragmentation_Strategy.h | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/ACE/ace/CDR_Stream.h b/ACE/ace/CDR_Stream.h index 0c6fa775af81f..8c913541d49fd 100644 --- a/ACE/ace/CDR_Stream.h +++ b/ACE/ace/CDR_Stream.h @@ -543,7 +543,7 @@ class ACE_Export ACE_OutputCDR * but for several elements @c memcpy should be more efficient, it * could be interesting to find the break even point and optimize * for that case, but that would be too platform dependent. - * + * * This is virtual to support GIOP fragmentation at the TAO layer. */ virtual ACE_CDR::Boolean write_array (const void *x, diff --git a/TAO/tao/CDR.cpp b/TAO/tao/CDR.cpp index cb1108511a5b3..53658629c0564 100644 --- a/TAO/tao/CDR.cpp +++ b/TAO/tao/CDR.cpp @@ -332,12 +332,12 @@ TAO_OutputCDR::write_16 (const ACE_CDR::LongDouble *x) if (avail < sizeof (CORBA::LongDouble)) { - const CORBA::ULongLong* ptr_8 + const CORBA::ULongLong* ptr_8 = reinterpret_cast (x); #if !defined (ACE_ENABLE_SWAP_ON_WRITE) - return + return ACE_OutputCDR::write_8 (ptr_8) && fragment_stream (ACE_CDR::LONGLONG_ALIGN, sizeof (CORBA::ULongLong)) && ACE_OutputCDR::write_8 (ptr_8 + 1); @@ -346,14 +346,14 @@ TAO_OutputCDR::write_16 (const ACE_CDR::LongDouble *x) if (!this->do_byte_swap_) { - return + return ACE_OutputCDR::write_8 (ptr_8) && fragment_stream (ACE_CDR::LONGLONG_ALIGN, sizeof (CORBA::ULongLong)) && ACE_OutputCDR::write_8 (ptr_8 + 1); } else { - return + return ACE_OutputCDR::write_8 (ptr_8 + 1) && fragment_stream (ACE_CDR::LONGLONG_ALIGN, sizeof (CORBA::ULongLong)) && ACE_OutputCDR::write_8 (ptr_8); @@ -365,7 +365,7 @@ TAO_OutputCDR::write_16 (const ACE_CDR::LongDouble *x) else { return ACE_OutputCDR::write_16 (x); - } + } } ACE_CDR::Boolean @@ -409,7 +409,7 @@ TAO_OutputCDR::write_array (const void *x, } else { - // We can write a batch of whole elements into the current fragment. + // We can write a batch of whole elements into the current fragment. lastBatch = (availableLength >= length); batchLength = (lastBatch ? length : availableLength); diff --git a/TAO/tao/On_Demand_Fragmentation_Strategy.cpp b/TAO/tao/On_Demand_Fragmentation_Strategy.cpp index 5d39da6619c9a..b2689cd7be475 100644 --- a/TAO/tao/On_Demand_Fragmentation_Strategy.cpp +++ b/TAO/tao/On_Demand_Fragmentation_Strategy.cpp @@ -22,7 +22,7 @@ TAO_On_Demand_Fragmentation_Strategy::TAO_On_Demand_Fragmentation_Strategy ( // + 8 for payload (including padding) // since fragments must be aligned on an 8 byte boundary. // Make it a multiple of 8 to avoid checking for this repeatedly - // at runtime. + // at runtime. } TAO_On_Demand_Fragmentation_Strategy::~TAO_On_Demand_Fragmentation_Strategy ( diff --git a/TAO/tao/On_Demand_Fragmentation_Strategy.h b/TAO/tao/On_Demand_Fragmentation_Strategy.h index 4914b86ccdb1d..ba83fc120840c 100644 --- a/TAO/tao/On_Demand_Fragmentation_Strategy.h +++ b/TAO/tao/On_Demand_Fragmentation_Strategy.h @@ -51,7 +51,7 @@ class TAO_On_Demand_Fragmentation_Strategy ACE_CDR::ULong pending_alignment, ACE_CDR::ULong pending_length); - virtual ACE_CDR::ULong available (TAO_OutputCDR & cdr, + virtual ACE_CDR::ULong available (TAO_OutputCDR & cdr, ACE_CDR::ULong pending_alignment); private: From 325c48afcbac93c777c1e8879be3249059e338e8 Mon Sep 17 00:00:00 2001 From: Joachim Achtzehnter Date: Fri, 11 Sep 2020 19:31:34 -0700 Subject: [PATCH 3/4] Addressed TAO style checks. --- ACE/ace/CDR_Stream.h | 2 - TAO/tao/CDR.cpp | 107 ++++++++++++++++++++++++------------------- TAO/tao/CDR.h | 4 +- 3 files changed, 63 insertions(+), 50 deletions(-) diff --git a/ACE/ace/CDR_Stream.h b/ACE/ace/CDR_Stream.h index 8c913541d49fd..cdfa52aaa621c 100644 --- a/ACE/ace/CDR_Stream.h +++ b/ACE/ace/CDR_Stream.h @@ -1065,7 +1065,6 @@ class ACE_Export ACE_InputCDR #endif /* ACE_HAS_MONITOR_POINTS==1 */ protected: - /// The start of the chain of message blocks, even though in the /// current version the chain always has length 1. ACE_Message_Block start_; @@ -1346,7 +1345,6 @@ class ACE_Export ACE_WChar_Codeset_Translator ACE_CDR::Octet minor_version (ACE_InputCDR& input); ACE_CDR::Octet major_version (ACE_OutputCDR& output); ACE_CDR::Octet minor_version (ACE_OutputCDR& output); - }; // @@ These operators should not be inlined since they force SString.h diff --git a/TAO/tao/CDR.cpp b/TAO/tao/CDR.cpp index 53658629c0564..c0512f67c5d83 100644 --- a/TAO/tao/CDR.cpp +++ b/TAO/tao/CDR.cpp @@ -377,50 +377,27 @@ TAO_OutputCDR::write_array (const void *x, if (length == 0) return 1; - const char* xPtr = static_cast (x); - - if (size > ACE_CDR::MAX_ALIGNMENT) + if (size <= ACE_CDR::MAX_ALIGNMENT) { - if (size != 16) - { - good_bit(false); - return 0; - } - - // may need to fragment in the middle of an element + const char* xPtr = static_cast (x); - if (!fragment_stream (align, ACE_CDR::MAX_ALIGNMENT)) + if (!fragment_stream (align, size)) return 0; while (true) { ACE_CDR::ULong availableBytes = fragment_bytes_available(align); ACE_CDR::ULong availableLength = availableBytes / size; - ACE_CDR::ULong batchLength; - bool lastBatch; - - if (availableLength == 0) - { - // This will fragment in the middle of the 16-byte element. - if (!write_16 (reinterpret_cast (xPtr))) - return 0; - batchLength = 1; - lastBatch = (length == 1); - } - else - { - // We can write a batch of whole elements into the current fragment. - lastBatch = (availableLength >= length); - batchLength = (lastBatch ? length : availableLength); - - if (!ACE_OutputCDR::write_array (xPtr, size, align, batchLength)) - return 0; - } + bool lastBatch = (availableLength >= length); + ACE_CDR::ULong batchLength = (lastBatch ? length : availableLength); + + if (!ACE_OutputCDR::write_array (xPtr, size, align, batchLength)) + return 0; if (lastBatch) return 1; - if (!fragment_stream (align, ACE_CDR::MAX_ALIGNMENT)) + if (!fragment_stream (align, size)) return 0; xPtr += batchLength * size; @@ -429,28 +406,65 @@ TAO_OutputCDR::write_array (const void *x, } else { - if (!fragment_stream (align, size)) + if (size == 16 && align == ACE_CDR::MAX_ALIGNMENT) + { + return write_array_16 (x, length); + } + else + { + good_bit(false); return 0; + } + } +} - while (true) - { - ACE_CDR::ULong availableBytes = fragment_bytes_available(align); - ACE_CDR::ULong availableLength = availableBytes / size; - bool lastBatch = (availableLength >= length); - ACE_CDR::ULong batchLength = (lastBatch ? length : availableLength); +ACE_CDR::Boolean TAO_OutputCDR::write_array_16 (const void *x, + ACE_CDR::ULong length) +{ + // may need to fragment in the middle of an element - if (!ACE_OutputCDR::write_array (xPtr, size, align, batchLength)) - return 0; + const ACE_CDR::LongDouble* xPtr + = static_cast (x); - if (lastBatch) - return 1; + if (!fragment_stream (ACE_CDR::MAX_ALIGNMENT, ACE_CDR::MAX_ALIGNMENT)) + return 0; - if (!fragment_stream (align, size)) + while (true) + { + ACE_CDR::ULong availableBytes + = fragment_bytes_available(ACE_CDR::MAX_ALIGNMENT); + ACE_CDR::ULong availableLength = availableBytes / 16; + ACE_CDR::ULong batchLength; + bool lastBatch; + + if (availableLength == 0) + { + // This will fragment in the middle of the 16-byte element. + if (!write_16 (reinterpret_cast (xPtr))) return 0; + batchLength = 1; + lastBatch = (length == 1); + } + else + { + // We can write a batch of whole elements into the current fragment. + lastBatch = (availableLength >= length); + batchLength = (lastBatch ? length : availableLength); - xPtr += batchLength * size; - length -= batchLength; + if (!ACE_OutputCDR::write_array (xPtr, 16, + ACE_CDR::MAX_ALIGNMENT, + batchLength)) + return 0; } + + if (lastBatch) + return 1; + + if (!fragment_stream (ACE_CDR::MAX_ALIGNMENT, ACE_CDR::MAX_ALIGNMENT)) + return 0; + + xPtr += batchLength; + length -= batchLength; } } @@ -519,7 +533,6 @@ TAO_InputCDR::throw_skel_exception (int error_num ) default : throw ::CORBA::MARSHAL(); - } } diff --git a/TAO/tao/CDR.h b/TAO/tao/CDR.h index 7b79e3ec390c6..55bb68211e849 100644 --- a/TAO/tao/CDR.h +++ b/TAO/tao/CDR.h @@ -257,7 +257,9 @@ class TAO_Export TAO_OutputCDR : public ACE_OutputCDR TAO_OutputCDR (const TAO_OutputCDR& rhs); TAO_OutputCDR& operator= (const TAO_OutputCDR& rhs); -private: +private: + ACE_CDR::Boolean write_array_16 (const void *x, + ACE_CDR::ULong length); /** * @name Outgoing GIOP Fragment Related Attributes * From 1599a0634518e75c013bd6401638d3ed40a51de5 Mon Sep 17 00:00:00 2001 From: Joachim Achtzehnter Date: Fri, 11 Sep 2020 19:40:17 -0700 Subject: [PATCH 4/4] Removed trailing white space. --- TAO/tao/CDR.cpp | 4 ++-- TAO/tao/CDR.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/TAO/tao/CDR.cpp b/TAO/tao/CDR.cpp index c0512f67c5d83..480489d12129c 100644 --- a/TAO/tao/CDR.cpp +++ b/TAO/tao/CDR.cpp @@ -451,8 +451,8 @@ ACE_CDR::Boolean TAO_OutputCDR::write_array_16 (const void *x, lastBatch = (availableLength >= length); batchLength = (lastBatch ? length : availableLength); - if (!ACE_OutputCDR::write_array (xPtr, 16, - ACE_CDR::MAX_ALIGNMENT, + if (!ACE_OutputCDR::write_array (xPtr, 16, + ACE_CDR::MAX_ALIGNMENT, batchLength)) return 0; } diff --git a/TAO/tao/CDR.h b/TAO/tao/CDR.h index 55bb68211e849..61a557ff062aa 100644 --- a/TAO/tao/CDR.h +++ b/TAO/tao/CDR.h @@ -257,7 +257,7 @@ class TAO_Export TAO_OutputCDR : public ACE_OutputCDR TAO_OutputCDR (const TAO_OutputCDR& rhs); TAO_OutputCDR& operator= (const TAO_OutputCDR& rhs); -private: +private: ACE_CDR::Boolean write_array_16 (const void *x, ACE_CDR::ULong length); /**