Skip to content

Commit 1ce9b0e

Browse files
committed
Apply PR #895: Refactor bmqu::AlignedPrinter to accept vector of strings
Complete implementation from PR #895: - Updated AlignedPrinter constructor to accept bsl::vector<bsl::string>* - Updated JsonPrinter to maintain API consistency - Replaced bsl::strlen() with bsl::string::length() calls - Added comprehensive unit test suite (bmqu_alignedprinter.t.cpp) - Updated all usage sites across bmqstoragetool, bmqtool, and mqbs modules - Optimized performance with emplace_back() instead of push_back() - Preserved const char* usage for hash/data pointers (not string literals) Files modified: - Core classes: bmqu_alignedprinter.h, bmqu_jsonprinter.h - Test suite: bmqu_alignedprinter.t.cpp (new) - Tools: m_bmqstoragetool_*, m_bmqtool_storageinspector.cpp - Storage: mqbs_filestoreprotocolprinter.* This addresses issue #564 by eliminating raw pointer usage for string literals while improving memory safety and following modern C++ practices. Signed-off-by: Kapil Jain <[email protected]>
1 parent 3942094 commit 1ce9b0e

11 files changed

+2163
-155
lines changed

src/applications/bmqstoragetool/m_bmqstoragetool_cslprinter.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,15 +246,15 @@ void HumanReadableCslPrinter::printSummaryResult(
246246
d_ostream << '\n'
247247
<< recordCount.d_updateCount
248248
<< " update record(s) found, including:" << '\n';
249-
bsl::vector<const char*> fields(d_allocator_p);
249+
bsl::vector<bsl::string> fields(d_allocator_p);
250250
bmqp_ctrlmsg::ClusterMessageChoice clusterMessageChoice(
251251
d_allocator_p);
252252
for (CslUpdateChoiceMap::const_iterator it =
253253
updateChoiceMap.begin();
254254
it != updateChoiceMap.end();
255255
++it) {
256256
clusterMessageChoice.makeSelection(it->first);
257-
fields.push_back(clusterMessageChoice.selectionName());
257+
fields.emplace_back(clusterMessageChoice.selectionName());
258258
}
259259
bmqu::AlignedPrinter printer(d_ostream, &fields);
260260
for (CslUpdateChoiceMap::const_iterator it =

src/applications/bmqstoragetool/m_bmqstoragetool_cslrecordprinter.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ template <typename PRINTER_TYPE>
9393
class CslRecordPrinter {
9494
private:
9595
bsl::ostream& d_ostream;
96-
bsl::vector<const char*> d_fields;
96+
bsl::vector<bsl::string> d_fields;
9797
bslma::ManagedPtr<PRINTER_TYPE> d_printer_mp;
9898
bslma::Allocator* d_allocator_p;
9999

src/applications/bmqstoragetool/m_bmqstoragetool_printer.cpp

Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ void printDataFileMeta(bsl::ostream& ostream,
9191
{
9292
BSLS_ASSERT_SAFE(dataFile_p && dataFile_p->isValid());
9393

94-
const bsl::vector<const char*> fields = {"BlazingMQ File Header",
94+
const bsl::vector<bsl::string> fields = {"BlazingMQ File Header",
9595
"Data File Header"};
9696

9797
PRINTER_TYPE1 printer(ostream, &fields);
@@ -121,7 +121,7 @@ void printJournalFileMeta(bsl::ostream& ostream,
121121
{
122122
BSLS_ASSERT_SAFE(journalFile_p && journalFile_p->isValid());
123123

124-
const bsl::vector<const char*> fields = {"BlazingMQ File Header",
124+
const bsl::vector<bsl::string> fields = {"BlazingMQ File Header",
125125
"Journal File Header",
126126
"Journal SyncPoint"};
127127

@@ -150,20 +150,20 @@ void printJournalFileMeta(bsl::ostream& ostream,
150150
s << '\n';
151151
{
152152
// Print journal-specific fields
153-
bsl::vector<const char*> fieldsSyncPoint(allocator);
153+
bsl::vector<bsl::string> fieldsSyncPoint(allocator);
154154
fieldsSyncPoint.reserve(12);
155-
fieldsSyncPoint.push_back("Last Valid Record Offset");
156-
fieldsSyncPoint.push_back("Record Type");
157-
fieldsSyncPoint.push_back("Record Timestamp");
158-
fieldsSyncPoint.push_back("Record Epoch");
159-
fieldsSyncPoint.push_back("Last Valid SyncPoint Offset");
160-
fieldsSyncPoint.push_back("SyncPoint Timestamp");
161-
fieldsSyncPoint.push_back("SyncPoint Epoch");
162-
fieldsSyncPoint.push_back("SyncPoint SeqNum");
163-
fieldsSyncPoint.push_back("SyncPoint Primary NodeId");
164-
fieldsSyncPoint.push_back("SyncPoint Primary LeaseId");
165-
fieldsSyncPoint.push_back("SyncPoint DataFileOffset (DWORDS)");
166-
fieldsSyncPoint.push_back("SyncPoint QlistFileOffset (WORDS)");
155+
fieldsSyncPoint.emplace_back("Last Valid Record Offset");
156+
fieldsSyncPoint.emplace_back("Record Type");
157+
fieldsSyncPoint.emplace_back("Record Timestamp");
158+
fieldsSyncPoint.emplace_back("Record Epoch");
159+
fieldsSyncPoint.emplace_back("Last Valid SyncPoint Offset");
160+
fieldsSyncPoint.emplace_back("SyncPoint Timestamp");
161+
fieldsSyncPoint.emplace_back("SyncPoint Epoch");
162+
fieldsSyncPoint.emplace_back("SyncPoint SeqNum");
163+
fieldsSyncPoint.emplace_back("SyncPoint Primary NodeId");
164+
fieldsSyncPoint.emplace_back("SyncPoint Primary LeaseId");
165+
fieldsSyncPoint.emplace_back("SyncPoint DataFileOffset (DWORDS)");
166+
fieldsSyncPoint.emplace_back("SyncPoint QlistFileOffset (WORDS)");
167167

168168
PRINTER_TYPE2 p(s, &fieldsSyncPoint);
169169
bsls::Types::Uint64 lastRecPos =
@@ -251,20 +251,20 @@ void printQueueDetails(bsl::ostream& ostream,
251251
const bsl::size_t appKeysCount = details.d_appDetailsMap.size();
252252

253253
// Setup fields to be displayed
254-
bsl::vector<const char*> fields(allocator);
254+
bsl::vector<bsl::string> fields(allocator);
255255
fields.reserve(8);
256-
fields.push_back("Queue Key");
256+
fields.emplace_back("Queue Key");
257257
if (!details.d_queueUri.empty()) {
258-
fields.push_back("Queue URI");
258+
fields.emplace_back("Queue URI");
259259
}
260-
fields.push_back("Total Records");
261-
fields.push_back("Num Queue Op Records");
262-
fields.push_back("Num Message Records");
263-
fields.push_back("Num Confirm Records");
260+
fields.emplace_back("Total Records");
261+
fields.emplace_back("Num Queue Op Records");
262+
fields.emplace_back("Num Message Records");
263+
fields.emplace_back("Num Confirm Records");
264264
if (appKeysCount > 1U) {
265-
fields.push_back("Num Records Per App");
265+
fields.emplace_back("Num Records Per App");
266266
}
267-
fields.push_back("Num Delete Records");
267+
fields.emplace_back("Num Delete Records");
268268

269269
{
270270
PRINTER_TYPE printer(ostream, &fields);
@@ -572,12 +572,12 @@ void HumanReadablePrinter::printQueueOpSummary(
572572
d_ostream << "\nTotal number of queueOp records: "
573573
<< queueOpRecordsCount << '\n';
574574

575-
bsl::vector<const char*> fields(d_allocator_p);
575+
bsl::vector<bsl::string> fields(d_allocator_p);
576576
fields.reserve(4);
577-
fields.push_back("Number of 'purge' operations");
578-
fields.push_back("Number of 'creation' operations");
579-
fields.push_back("Number of 'deletion' operations");
580-
fields.push_back("Number of 'addition' operations");
577+
fields.emplace_back("Number of 'purge' operations");
578+
fields.emplace_back("Number of 'creation' operations");
579+
fields.emplace_back("Number of 'deletion' operations");
580+
fields.emplace_back("Number of 'addition' operations");
581581
bmqu::AlignedPrinter printer(d_ostream, &fields);
582582
printer << queueOpCountsVec[mqbs::QueueOpType::e_PURGE]
583583
<< queueOpCountsVec[mqbs::QueueOpType::e_CREATION]
@@ -887,13 +887,13 @@ void JsonPrinter::printQueueOpSummary(
887887
{
888888
BSLS_ASSERT_SAFE(queueOpCountsVec.size() > mqbs::QueueOpType::e_ADDITION);
889889
closeBraceIfOpen();
890-
bsl::vector<const char*> fields(d_allocator_p);
890+
bsl::vector<bsl::string> fields(d_allocator_p);
891891
fields.reserve(5);
892-
fields.push_back("TotalQueueOperationsNumber");
893-
fields.push_back("PurgeOperationsNumber");
894-
fields.push_back("CreationOperationsNumber");
895-
fields.push_back("DeletionOperationsNumber");
896-
fields.push_back("AdditionOperationsNumber");
892+
fields.emplace_back("TotalQueueOperationsNumber");
893+
fields.emplace_back("PurgeOperationsNumber");
894+
fields.emplace_back("CreationOperationsNumber");
895+
fields.emplace_back("DeletionOperationsNumber");
896+
fields.emplace_back("AdditionOperationsNumber");
897897

898898
bmqu::JsonPrinter<true, false, 0, 2> printer(d_ostream, &fields);
899899
printer << queueOpRecordsCount

src/applications/bmqstoragetool/m_bmqstoragetool_recordprinter.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ template <typename PRINTER_TYPE>
6565
class RecordDetailsPrinter {
6666
private:
6767
bsl::ostream& d_ostream;
68-
bsl::vector<const char*> d_fields;
68+
bsl::vector<bsl::string> d_fields;
6969
bslma::ManagedPtr<PRINTER_TYPE> d_printer_mp;
7070
bslma::Allocator* d_allocator_p;
7171

src/applications/bmqtool/m_bmqtool_storageinspector.cpp

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -465,19 +465,19 @@ void StorageInspector::processCommand(
465465

466466
// Print journal-specific fields
467467
BALL_LOG_OUTPUT_STREAM << "Journal SyncPoint:\n";
468-
bsl::vector<const char*> fields;
469-
fields.push_back("Last Valid Record Offset");
470-
fields.push_back("Record Type");
471-
fields.push_back("Record Timestamp");
472-
fields.push_back("Record Epoch");
473-
fields.push_back("Last Valid SyncPoint Offset");
474-
fields.push_back("SyncPoint Timestamp");
475-
fields.push_back("SyncPoint Epoch");
476-
fields.push_back("SyncPoint SeqNum");
477-
fields.push_back("SyncPoint Primary NodeId");
478-
fields.push_back("SyncPoint Primary LeaseId");
479-
fields.push_back("SyncPoint DataFileOffset (DWORDS)");
480-
fields.push_back("SyncPoint QlistFileOffset (WORDS)");
468+
bsl::vector<bsl::string> fields;
469+
fields.emplace_back("Last Valid Record Offset");
470+
fields.emplace_back("Record Type");
471+
fields.emplace_back("Record Timestamp");
472+
fields.emplace_back("Record Epoch");
473+
fields.emplace_back("Last Valid SyncPoint Offset");
474+
fields.emplace_back("SyncPoint Timestamp");
475+
fields.emplace_back("SyncPoint Epoch");
476+
fields.emplace_back("SyncPoint SeqNum");
477+
fields.emplace_back("SyncPoint Primary NodeId");
478+
fields.emplace_back("SyncPoint Primary LeaseId");
479+
fields.emplace_back("SyncPoint DataFileOffset (DWORDS)");
480+
fields.emplace_back("SyncPoint QlistFileOffset (WORDS)");
481481

482482
bmqu::AlignedPrinter printer(BALL_LOG_OUTPUT_STREAM, &fields);
483483
bsls::Types::Uint64 lastRecPos =
@@ -578,10 +578,10 @@ void StorageInspector::processCommand(
578578
BALL_LOG_OUTPUT_STREAM << "Queue #" << qnum << "\n";
579579
const QueueRecord& qr = cit->second;
580580

581-
bsl::vector<const char*> fields;
582-
fields.push_back("Queue URI");
583-
fields.push_back("QueueKey");
584-
fields.push_back("Number of AppIds");
581+
bsl::vector<bsl::string> fields;
582+
fields.emplace_back("Queue URI");
583+
fields.emplace_back("QueueKey");
584+
fields.emplace_back("Number of AppIds");
585585

586586
bmqu::AlignedPrinter printer(BALL_LOG_OUTPUT_STREAM, &fields);
587587
printer << cit->first << qr.d_queueKey << qr.d_appIds.size();
@@ -592,9 +592,9 @@ void StorageInspector::processCommand(
592592
for (unsigned int i = 0; i < appRecs.size(); ++i) {
593593
const AppIdRecord& ar = appRecs[i];
594594
BALL_LOG_OUTPUT_STREAM << " AppId #" << i + 1 << "\n";
595-
bsl::vector<const char*> f;
596-
f.push_back("AppId");
597-
f.push_back("AppKey");
595+
bsl::vector<bsl::string> f;
596+
f.emplace_back("AppId");
597+
f.emplace_back("AppKey");
598598

599599
const int indent = 8;
600600
bmqu::AlignedPrinter p(BALL_LOG_OUTPUT_STREAM, &f, indent);

src/groups/bmq/bmqu/bmqu_alignedprinter.h

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,10 @@
2929
///-----
3030
// First, specify field names for printer:
3131
//..
32-
// bsl::vector<const char*> fields;
33-
// fields.push_back("Queue URI");
34-
// fields.push_back("QueueKey");
35-
// fields.push_back("Number of AppIds");
32+
// bsl::vector<bsl::string> fields;
33+
// fields.emplace_back("Queue URI");
34+
// fields.emplace_back("QueueKey");
35+
// fields.emplace_back("Number of AppIds");
3636
//..
3737
//
3838
// Next, create an instance of bmqu::AlignedPrinter:
@@ -71,7 +71,7 @@ class AlignedPrinter {
7171
private:
7272
// DATA
7373
bsl::ostream& d_ostream;
74-
const bsl::vector<const char*>* d_fields_p;
74+
const bsl::vector<bsl::string>* d_fields_p;
7575
int d_indent;
7676
int d_width;
7777
unsigned int d_counter;
@@ -89,7 +89,7 @@ class AlignedPrinter {
8989
/// is undefined unless `indent` >= 0 and at least one field is present
9090
/// in the `fields`.
9191
AlignedPrinter(bsl::ostream& stream,
92-
const bsl::vector<const char*>* fields,
92+
const bsl::vector<bsl::string>* fields,
9393
int indent = 4);
9494

9595
// MANIPULATORS
@@ -110,7 +110,7 @@ class AlignedPrinter {
110110
// --------------
111111

112112
inline AlignedPrinter::AlignedPrinter(bsl::ostream& stream,
113-
const bsl::vector<const char*>* fields,
113+
const bsl::vector<bsl::string>* fields,
114114
int indent)
115115
: d_ostream(stream)
116116
, d_fields_p(fields)
@@ -121,9 +121,9 @@ inline AlignedPrinter::AlignedPrinter(bsl::ostream& stream,
121121
BSLS_ASSERT_SAFE(0 <= d_indent);
122122
BSLS_ASSERT_SAFE(0 < d_fields_p->size());
123123

124-
int maxLen = static_cast<int>(bsl::strlen((*d_fields_p)[0]));
124+
int maxLen = static_cast<int>((*d_fields_p)[0].length());
125125
for (unsigned int i = 1; i < d_fields_p->size(); ++i) {
126-
int len = static_cast<int>(bsl::strlen((*d_fields_p)[i]));
126+
int len = static_cast<int>((*d_fields_p)[i].length());
127127
if (maxLen < len) {
128128
maxLen = len;
129129
}
@@ -138,8 +138,8 @@ inline AlignedPrinter& AlignedPrinter::operator<<(const TYPE& value)
138138
BSLS_ASSERT_SAFE(d_counter < d_fields_p->size());
139139

140140
d_ostream << bsl::setw(d_indent) << ' ' << (*d_fields_p)[d_counter]
141-
<< bsl::setw(static_cast<int>(
142-
d_width - bsl::strlen((*d_fields_p)[d_counter])))
141+
<< bsl::setw(static_cast<int>(d_width -
142+
(*d_fields_p)[d_counter].length()))
143143
<< ": " << value << '\n';
144144

145145
++d_counter;

0 commit comments

Comments
 (0)