Review fixes

Signed-off-by: Evgeny Malygin <[email protected]>

src/groups/bmq/bmqa/bmqa_mocksession.h

@@ -1024,7 +1024,7 @@ class MockSession : public AbstractSession {
 
     // DATA
 
-    /// Buffer factory
+    /// Buffer factory used to build Blobs with `d_blobSpPool`
     bdlbb::PooledBlobBufferFactory d_blobBufferFactory;
 
     /// Pool of shared pointers to blobs

src/groups/bmq/bmqimp/bmqimp_application.h

@@ -259,11 +259,6 @@ class Application {
 
     // MANIPULATORS
 
-    /// Return a pointer to the blob buffer factory used by this instance.
-    /// Note that lifetime of the pointed-to buffer factory is bound by this
-    /// instance.
-    bdlbb::BlobBufferFactory* bufferFactory();
-
     /// Return a pointer to the blob shared pointer pool used by this instance.
     /// Note that lifetime of the pointed-to pool is bound by this instance.
     BlobSpPool* blobSpPool();
@@ -327,11 +322,6 @@ inline bool Application::isStarted() const
             state == bmqimp::BrokerSession::State::e_RECONNECTING);
 }
 
-inline bdlbb::BlobBufferFactory* Application::bufferFactory()
-{
-    return &d_blobBufferFactory;
-}
-
 inline Application::BlobSpPool* Application::blobSpPool()
 {
     return &d_blobSpPool;

src/groups/bmq/bmqp/bmqp_ackeventbuilder.cpp

@@ -37,13 +37,15 @@ namespace bmqp {
 AckEventBuilder::AckEventBuilder(BlobSpPool*       blobSpPool_p,
                                  bslma::Allocator* allocator)
 : d_blobSpPool_p(blobSpPool_p)
-, d_blob_sp(0, allocator)  // initialized in `reset()`
-, d_emptyBlob_sp(blobSpPool_p->getObject())
+, d_blob_sp(0, allocator)       // initialized in `reset()`
+, d_emptyBlob_sp(0, allocator)  // initialized later in constructor
 , d_msgCount(0)
 {
     // PRECONDITIONS
     BSLS_ASSERT_SAFE(blobSpPool_p);
 
+    d_emptyBlob_sp = blobSpPool_p->getObject();
+
     // Assume that items built with the given `blobSpPool_p` either all have or
     // all don't have buffer factory, and check it once for `d_emptyBlob_sp`.
     // We require this since we do `Blob::setLength`:

src/groups/bmq/bmqp/bmqp_confirmeventbuilder.cpp

@@ -37,13 +37,15 @@ namespace bmqp {
 ConfirmEventBuilder::ConfirmEventBuilder(BlobSpPool*       blobSpPool_p,
                                          bslma::Allocator* allocator)
 : d_blobSpPool_p(blobSpPool_p)
-, d_blob_sp(0, allocator)  // initialized in `reset()`
-, d_emptyBlob_sp(blobSpPool_p->getObject())
+, d_blob_sp(0, allocator)       // initialized in `reset()`
+, d_emptyBlob_sp(0, allocator)  // initialized later in constructor
 , d_msgCount(0)
 {
     // PRECONDITIONS
     BSLS_ASSERT_SAFE(blobSpPool_p);
 
+    d_emptyBlob_sp = blobSpPool_p->getObject();
+
     // Assume that items built with the given `blobSpPool_p` either all have or
     // all don't have buffer factory, and check it once for `d_emptyBlob_sp`.
     // We require this since we do `Blob::setLength`:

src/groups/bmq/bmqp/bmqp_protocolutil.h

@@ -406,8 +406,8 @@ struct ProtocolUtil {
     buildEvent(ACTION_FUNCTOR_TYPE&   actionCb,
                OVERFLOW_FUNCTOR_TYPE& overflowCb);
 
-    /// Encode Receipt into the specified `blob` for the specified
-    /// `partitionId`, `primaryLeaseId`, and `sequenceNumber`.
+    /// Encode Receipt into the specified `blob` (expected to be empty) for
+    /// the specified `partitionId`, `primaryLeaseId`, and `sequenceNumber`.
     static void buildReceipt(bdlbb::Blob*        blob,
                              int                 partitionId,
                              unsigned int        primaryLeaseId,
@@ -732,7 +732,8 @@ inline void ProtocolUtil::buildReceipt(bdlbb::Blob*        blob,
                                        unsigned int        primaryLeaseId,
                                        bsls::Types::Uint64 sequenceNumber)
 {
-    blob->removeAll();
+    // PRECONDITIONS
+    BSLS_ASSERT_SAFE(0 == blob->length());
 
     blob->setLength(sizeof(EventHeader) + sizeof(ReplicationReceipt));
 

src/groups/bmq/bmqp/bmqp_pusheventbuilder.cpp

@@ -151,7 +151,6 @@ PushEventBuilder::PushEventBuilder(BlobSpPool*       blobSpPool_p,
 : d_allocator_p(bslma::Default::allocator(allocator))
 , d_blobSpPool_p(blobSpPool_p)
 , d_blob_sp(0, allocator)  // initialized in `reset()`
-, d_emptyBlob_sp(blobSpPool_p->getObject())
 , d_msgCount(0)
 , d_options()
 , d_currPushHeader()
@@ -160,10 +159,10 @@ PushEventBuilder::PushEventBuilder(BlobSpPool*       blobSpPool_p,
     BSLS_ASSERT_SAFE(blobSpPool_p);
 
     // Assume that items built with the given `blobSpPool_p` either all have or
-    // all don't have buffer factory, and check it once for `d_emptyBlob_sp`.
+    // all don't have buffer factory, and check it once for a sample blob.
     // We require this since we do `Blob::setLength`:
     BSLS_ASSERT_SAFE(
-        NULL != d_emptyBlob_sp->factory() &&
+        NULL != d_blobSpPool_p->getObject()->factory() &&
         "Passed BlobSpPool must build Blobs with set BlobBufferFactory");
 
     reset();
@@ -406,11 +405,8 @@ PushEventBuilder::addMsgGroupIdOption(const Protocol::MsgGroupId& msgGroupId)
 // ACCESSORS
 const bsl::shared_ptr<bdlbb::Blob>& PushEventBuilder::blob() const
 {
-    // Empty event
-    if (BSLS_PERFORMANCEHINT_PREDICT_UNLIKELY(messageCount() == 0)) {
-        BSLS_PERFORMANCEHINT_UNLIKELY_HINT;
-        return d_emptyBlob_sp;  // RETURN
-    }
+    // PRECONDITIONS
+    BSLS_ASSERT_SAFE(d_blob_sp->length() <= EventHeader::k_MAX_SIZE_SOFT);
 
     // Fix packet's length in header now that we know it ..  Following is valid
     // (see comment in reset)

src/groups/bmq/bmqp/bmqp_pusheventbuilder.h

@@ -103,9 +103,6 @@ class PushEventBuilder {
     /// `mutable` to skip writing the length until the blob is retrieved.
     mutable bsl::shared_ptr<bdlbb::Blob> d_blob_sp;
 
-    /// Empty blob to be returned when no messages were added to this builder.
-    bsl::shared_ptr<bdlbb::Blob> d_emptyBlob_sp;
-
     int d_msgCount;
     // number of messages currently in
     // the event.

src/groups/bmq/bmqp/bmqp_recoveryeventbuilder.cpp

@@ -43,17 +43,16 @@ RecoveryEventBuilder::RecoveryEventBuilder(BlobSpPool*       blobSpPool_p,
                                            bslma::Allocator* allocator)
 : d_blobSpPool_p(blobSpPool_p)
 , d_blob_sp(0, allocator)  // initialized in `reset()`
-, d_emptyBlob_sp(blobSpPool_p->getObject())
 , d_msgCount(0)
 {
     // PRECONDITIONS
     BSLS_ASSERT_SAFE(blobSpPool_p);
 
     // Assume that items built with the given `blobSpPool_p` either all have or
-    // all don't have buffer factory, and check it once for `d_emptyBlob_sp`.
+    // all don't have buffer factory, and check it once for a sample blob.
     // We require this since we do `Blob::setLength`:
     BSLS_ASSERT_SAFE(
-        NULL != d_emptyBlob_sp->factory() &&
+        NULL != blobSpPool_p->getObject()->factory() &&
         "Passed BlobSpPool must build Blobs with set BlobBufferFactory");
 
     reset();
@@ -156,10 +155,8 @@ RecoveryEventBuilder::packMessage(unsigned int                partitionId,
 
 const bsl::shared_ptr<bdlbb::Blob>& RecoveryEventBuilder::blob() const
 {
-    if (BSLS_PERFORMANCEHINT_PREDICT_UNLIKELY(messageCount() == 0)) {
-        BSLS_PERFORMANCEHINT_UNLIKELY_HINT;
-        return d_emptyBlob_sp;  // RETURN
-    }
+    // PRECONDITIONS
+    BSLS_ASSERT_SAFE(d_blob_sp->length() <= EventHeader::k_MAX_SIZE_SOFT);
 
     // Fix packet's length in header now that we know it ..  Following is valid
     // (see comment in reset)

src/groups/bmq/bmqp/bmqp_recoveryeventbuilder.h

@@ -99,9 +99,6 @@ class RecoveryEventBuilder BSLS_CPP11_FINAL {
     /// `mutable` to skip writing the length until the blob is retrieved.
     mutable bsl::shared_ptr<bdlbb::Blob> d_blob_sp;
 
-    /// Empty blob to be returned when no messages were added to this builder.
-    bsl::shared_ptr<bdlbb::Blob> d_emptyBlob_sp;
-
     int d_msgCount;  // number of messages currently in the
                      // event
 
@@ -156,9 +153,7 @@ class RecoveryEventBuilder BSLS_CPP11_FINAL {
     /// Return the number of messages currently in the event being built.
     int messageCount() const;
 
-    /// Return a reference to the shared pointer to the built Blob.  If no
-    /// messages were added, the Blob object under this reference will be
-    /// empty.
+    /// Return a reference to the shared pointer to the built Blob.
     /// Note that this accessor exposes an internal shared pointer object, and
     /// it is the user's responsibility to make a copy of it if it needs to be
     /// passed and kept in another thread while this builder object is used.

src/groups/bmq/bmqp/bmqp_rejecteventbuilder.cpp

@@ -37,18 +37,20 @@ namespace bmqp {
 RejectEventBuilder::RejectEventBuilder(BlobSpPool*       blobSpPool_p,
                                        bslma::Allocator* allocator)
 : d_blobSpPool_p(blobSpPool_p)
-, d_blob_sp(0, allocator)  // initialized in `reset()`
-, d_emptyBlob_sp(blobSpPool_p->getObject())
+, d_blob_sp(0, allocator)       // initialized in `reset()`
+, d_emptyBlob_sp(0, allocator)  // initialized later in constructor
 , d_msgCount(0)
 {
     // PRECONDITIONS
     BSLS_ASSERT_SAFE(blobSpPool_p);
 
+    d_emptyBlob_sp = blobSpPool_p->getObject();
+
     // Assume that items built with the given `blobSpPool_p` either all have or
     // all don't have buffer factory, and check it once for `d_emptyBlob_sp`.
     // We require this since we do `Blob::setLength`:
     BSLS_ASSERT_SAFE(
-        NULL != d_emptyBlob_sp->factory() &&
+        NULL != blobSpPool_p->getObject()->factory() &&
         "Passed BlobSpPool must build Blobs with set BlobBufferFactory");
 
     reset();

src/groups/bmq/bmqp/bmqp_storageeventbuilder.cpp

@@ -110,15 +110,17 @@ StorageEventBuilder::StorageEventBuilder(int storageProtocolVersion,
 : d_blobSpPool_p(blobSpPool_p)
 , d_storageProtocolVersion(storageProtocolVersion)
 , d_eventType(eventType)
-, d_blob_sp(0, allocator)  // initialized in `reset()`
-, d_emptyBlob_sp(blobSpPool_p->getObject())
+, d_blob_sp(0, allocator)       // initialized in `reset()`
+, d_emptyBlob_sp(0, allocator)  // initialized later in constructor
 , d_msgCount(0)
 {
     // PRECONDITIONS
     BSLS_ASSERT_SAFE(blobSpPool_p);
     BSLS_ASSERT_SAFE(EventType::e_STORAGE == eventType ||
                      EventType::e_PARTITION_SYNC == eventType);
 
+    d_emptyBlob_sp = blobSpPool_p->getObject();
+
     // Assume that items built with the given `blobSpPool_p` either all have or
     // all don't have buffer factory, and check it once for `d_emptyBlob_sp`.
     // We require this since we do `Blob::setLength`:
@@ -192,6 +194,9 @@ StorageEventBuilder::packMessageRaw(const bdlbb::Blob&        blob,
 
 const bsl::shared_ptr<bdlbb::Blob>& StorageEventBuilder::blob() const
 {
+    // PRECONDITIONS
+    BSLS_ASSERT_SAFE(d_blob_sp->length() <= EventHeader::k_MAX_SIZE_SOFT);
+
     if (BSLS_PERFORMANCEHINT_PREDICT_UNLIKELY(messageCount() == 0)) {
         BSLS_PERFORMANCEHINT_UNLIKELY_HINT;
         return d_emptyBlob_sp;  // RETURN