From 7124b4ee2e046dea1fa0513aed0d6d8923fafd6a Mon Sep 17 00:00:00 2001 From: Anton Korobeynikov Date: Fri, 7 Jun 2024 18:16:43 -0700 Subject: [PATCH] Format-related string fixes and refactorings (#4704) * Use abseil StrFormat in SourceFile * Simplify source file: do not create cstrings for something transient * Switch to cord for source code builder * Switch to statically-checked format string printing. Uncovered and fixed few bugs here and there. * Switch from Utils::printf_format to abseil formatting. Overall cstring's are misused everywhere in this code. But this is a subject of different task. * Get rid of unsafe and error-prone printf_format * Add missed conversion behind #ifdef --- backends/ebpf/ebpfControl.cpp | 8 +- backends/ebpf/ebpfDeparser.cpp | 8 +- backends/ebpf/ebpfParser.cpp | 39 +++++----- backends/ebpf/ebpfTable.cpp | 38 +++++---- backends/ebpf/psa/ebpfPipeline.cpp | 45 +++++------ backends/ebpf/psa/ebpfPsaControl.cpp | 2 +- backends/ebpf/psa/ebpfPsaDeparser.cpp | 2 +- backends/ebpf/psa/ebpfPsaGen.h | 6 +- backends/ebpf/psa/ebpfPsaTable.cpp | 7 +- backends/ebpf/psa/externs/ebpfPsaCounter.cpp | 18 ++--- backends/ebpf/psa/externs/ebpfPsaDigest.cpp | 2 +- .../ebpf/psa/externs/ebpfPsaHashAlgorithm.cpp | 4 +- backends/ebpf/psa/externs/ebpfPsaRegister.cpp | 11 ++- .../externs/ebpfPsaTableImplementation.cpp | 17 ++-- backends/ebpf/target.cpp | 9 ++- backends/tc/ebpfCodeGen.cpp | 77 +++++++++---------- frontends/p4/toP4/toP4.cpp | 4 +- ir/visitor.cpp | 2 +- lib/error_reporter.h | 2 + lib/sourceCodeBuilder.h | 37 +++++---- lib/source_file.cpp | 76 +++++++++--------- lib/source_file.h | 3 +- lib/stringify.cpp | 28 ------- lib/stringify.h | 4 - test/CMakeLists.txt | 1 - test/gtest/format_test.cpp | 3 - test/gtest/source_file_test.cpp | 4 +- test/gtest/stringify.cpp | 69 ----------------- 28 files changed, 208 insertions(+), 318 deletions(-) delete mode 100644 test/gtest/stringify.cpp diff --git a/backends/ebpf/ebpfControl.cpp b/backends/ebpf/ebpfControl.cpp index ad4563a8aa..8e4fdc3b85 100644 --- a/backends/ebpf/ebpfControl.cpp +++ b/backends/ebpf/ebpfControl.cpp @@ -143,7 +143,7 @@ bool ControlBodyTranslator::preorder(const IR::MethodCallExpression *expression) BUG_CHECK(expression->arguments->size() == 0, "%1%: unexpected arguments for action call", expression); cstring msg = - Util::printf_format("Control: explicit calling action %s()", ac->action->name.name); + absl::StrFormat("Control: explicit calling action %s()", ac->action->name.name); builder->target->emitTraceMessage(builder, msg.c_str()); visit(ac->action->body); return false; @@ -271,7 +271,7 @@ void ControlBodyTranslator::compileEmit(const IR::Vector *args) { // Increment header pointer builder->emitIndent(); - builder->appendFormat("%s += BYTES(%s);", program->headerStartVar.c_str(), width); + builder->appendFormat("%s += BYTES(%d);", program->headerStartVar.c_str(), width); builder->newline(); builder->blockEnd(true); @@ -304,7 +304,7 @@ void ControlBodyTranslator::processApply(const P4::ApplyMethod *method) { auto table = control->getTable(method->object->getName().name); BUG_CHECK(table != nullptr, "No table for %1%", method->expr); - msgStr = Util::printf_format("Control: applying %s", method->object->getName().name); + msgStr = absl::StrFormat("Control: applying %s", method->object->getName().name); builder->target->emitTraceMessage(builder, msgStr.c_str()); builder->emitIndent(); @@ -393,7 +393,7 @@ void ControlBodyTranslator::processApply(const P4::ApplyMethod *method) { builder->blockEnd(true); builder->blockEnd(true); - msgStr = Util::printf_format("Control: %s applied", method->object->getName().name); + msgStr = absl::StrFormat("Control: %s applied", method->object->getName().name); builder->target->emitTraceMessage(builder, msgStr.c_str()); } diff --git a/backends/ebpf/ebpfDeparser.cpp b/backends/ebpf/ebpfDeparser.cpp index 41a8b3d424..64a53004b5 100644 --- a/backends/ebpf/ebpfDeparser.cpp +++ b/backends/ebpf/ebpfDeparser.cpp @@ -133,7 +133,7 @@ void DeparserHdrEmitTranslator::processMethod(const P4::ExternMethod *method) { "Header %1% size %2% is not a multiple of 8 bits.", expr, width); return; } - msgStr = Util::printf_format("Deparser: emitting header %s", expr->toString().c_str()); + msgStr = absl::StrFormat("Deparser: emitting header %s", expr->toString().c_str()); builder->target->emitTraceMessage(builder, msgStr.c_str()); builder->emitIndent(); @@ -201,13 +201,13 @@ void DeparserHdrEmitTranslator::emitField(CodeBuilder *builder, cstring field, visit(hdrExpr); builder->appendFormat(".%s", field.c_str()); builder->endOfStatement(true); - msgStr = Util::printf_format("Deparser: emitting field %s=0x%%llx (%u bits)", field, - widthToEmit); + msgStr = absl::StrFormat("Deparser: emitting field %s=0x%%llx (%u bits)", field, + widthToEmit); builder->target->emitTraceMessage(builder, msgStr.c_str(), 1, "tmp"); builder->blockEnd(true); } } else { - msgStr = Util::printf_format("Deparser: emitting field %s (%u bits)", field, widthToEmit); + msgStr = absl::StrFormat("Deparser: emitting field %s (%u bits)", field, widthToEmit); builder->target->emitTraceMessage(builder, msgStr.c_str()); } diff --git a/backends/ebpf/ebpfParser.cpp b/backends/ebpf/ebpfParser.cpp index 26bdb52a18..377908b077 100644 --- a/backends/ebpf/ebpfParser.cpp +++ b/backends/ebpf/ebpfParser.cpp @@ -24,9 +24,9 @@ limitations under the License. namespace EBPF { void StateTranslationVisitor::compileLookahead(const IR::Expression *destination) { - cstring msgStr = Util::printf_format("Parser: lookahead for %s %s", - state->parser->typeMap->getType(destination)->toString(), - destination->toString()); + cstring msgStr = absl::StrFormat("Parser: lookahead for %s %s", + state->parser->typeMap->getType(destination)->toString(), + destination->toString()); builder->target->emitTraceMessage(builder, msgStr.c_str()); builder->emitIndent(); @@ -49,8 +49,8 @@ void StateTranslationVisitor::compileAdvance(const P4::ExternMethod *extMethod) if (cnst) { cstring argStr = cstring::to_cstring(cnst->asUnsigned()); cstring offsetStr = - Util::printf_format("%s - (u8*)%s + BYTES(%s)", state->parser->program->headerStartVar, - state->parser->program->packetStartVar, argStr); + absl::StrFormat("%s - (u8*)%s + BYTES(%s)", state->parser->program->headerStartVar, + state->parser->program->packetStartVar, argStr); builder->target->emitTraceMessage(builder, "Parser (advance): check pkt_len=%%d < " "last_read_byte=%%d", @@ -115,8 +115,8 @@ void StateTranslationVisitor::compileVerify(const IR::MethodCallExpression *expr errorMember->member.name); builder->endOfStatement(true); - cstring msg = Util::printf_format("Verify: condition failed, parser_error=%%u (%s)", - errorMember->member.name); + cstring msg = absl::StrFormat("Verify: condition failed, parser_error=%%u (%s)", + errorMember->member.name); builder->target->emitTraceMessage(builder, msg.c_str(), 1, state->parser->program->errorVar.c_str()); @@ -156,10 +156,9 @@ bool StateTranslationVisitor::preorder(const IR::ParserState *parserState) { builder->spc(); builder->blockStart(); - cstring msgStr = - Util::printf_format("Parser: state %s (curr_offset=%%d)", parserState->name.name); - cstring offsetStr = Util::printf_format("%s - (u8*)%s", state->parser->program->headerStartVar, - state->parser->program->packetStartVar); + cstring msgStr = absl::StrFormat("Parser: state %s (curr_offset=%%d)", parserState->name.name); + cstring offsetStr = absl::StrFormat("%s - (u8*)%s", state->parser->program->headerStartVar, + state->parser->program->packetStartVar); builder->target->emitTraceMessage(builder, msgStr.c_str(), 1, offsetStr.c_str()); visit(parserState->components, "components"); @@ -283,7 +282,7 @@ void StateTranslationVisitor::compileExtractField(const IR::Expression *expr, cstring msgStr; cstring fieldName = field->name.name; - msgStr = Util::printf_format("Parser: extracting field %s", fieldName); + msgStr = absl::StrFormat("Parser: extracting field %s", fieldName); builder->target->emitTraceMessage(builder, msgStr.c_str()); if (widthToExtract <= 64) { @@ -390,13 +389,13 @@ void StateTranslationVisitor::compileExtractField(const IR::Expression *expr, expr->to()->expr->to()->path->name.name)) { exprStr = exprStr.replace("."_cs, "->"_cs); } - cstring tmp = Util::printf_format("(unsigned long long) %s.%s", exprStr, fieldName); + cstring tmp = absl::StrFormat("(unsigned long long) %s.%s", exprStr, fieldName); - msgStr = Util::printf_format("Parser: extracted %s=0x%%llx (%u bits)", fieldName, - widthToExtract); + msgStr = + absl::StrFormat("Parser: extracted %s=0x%%llx (%u bits)", fieldName, widthToExtract); builder->target->emitTraceMessage(builder, msgStr.c_str(), 1, tmp.c_str()); } else { - msgStr = Util::printf_format("Parser: extracted %s (%u bits)", fieldName, widthToExtract); + msgStr = absl::StrFormat("Parser: extracted %s (%u bits)", fieldName, widthToExtract); builder->target->emitTraceMessage(builder, msgStr.c_str()); } } @@ -421,8 +420,8 @@ void StateTranslationVisitor::compileExtract(const IR::Expression *destination) auto program = state->parser->program; - cstring offsetStr = Util::printf_format("(%s - (u8*)%s) + BYTES(%s)", program->headerStartVar, - program->packetStartVar, cstring::to_cstring(width)); + cstring offsetStr = absl::StrFormat("(%s - (u8*)%s) + BYTES(%s)", program->headerStartVar, + program->packetStartVar, cstring::to_cstring(width)); builder->target->emitTraceMessage(builder, "Parser: check pkt_len=%d >= last_read_byte=%d", 2, program->lengthVar.c_str(), offsetStr.c_str()); @@ -465,7 +464,7 @@ void StateTranslationVisitor::compileExtract(const IR::Expression *destination) builder->newline(); builder->blockEnd(true); - msgStr = Util::printf_format("Parser: extracting header %s", destination->toString()); + msgStr = absl::StrFormat("Parser: extracting header %s", destination->toString()); builder->target->emitTraceMessage(builder, msgStr.c_str()); builder->newline(); @@ -495,7 +494,7 @@ void StateTranslationVisitor::compileExtract(const IR::Expression *destination) builder->appendFormat("%s += BYTES(%u);", program->headerStartVar.c_str(), width); builder->newline(); - msgStr = Util::printf_format("Parser: extracted %s", destination->toString()); + msgStr = absl::StrFormat("Parser: extracted %s", destination->toString()); builder->target->emitTraceMessage(builder, msgStr.c_str()); builder->newline(); diff --git a/backends/ebpf/ebpfTable.cpp b/backends/ebpf/ebpfTable.cpp index 2c0c866f4c..428781dafe 100644 --- a/backends/ebpf/ebpfTable.cpp +++ b/backends/ebpf/ebpfTable.cpp @@ -44,9 +44,7 @@ bool ActionTranslationVisitor::isActionParameter(const IR::PathExpression *expre cstring ActionTranslationVisitor::getParamInstanceName(const IR::Expression *expression) const { cstring actionName = EBPFObject::externalName(action); - auto paramStr = - Util::printf_format("%s->u.%s.%s", valueName, actionName, expression->toString()); - return paramStr; + return absl::StrFormat("%s->u.%s.%s", valueName, actionName, expression->toString()); } bool ActionTranslationVisitor::preorder(const IR::P4Action *act) { @@ -512,12 +510,12 @@ void EBPFTable::emitKey(CodeBuilder *builder, cstring keyName) { cstring msgStr, varStr; if (memcpy) { - msgStr = Util::printf_format("Control: key %s", c->expression->toString()); + msgStr = absl::StrFormat("Control: key %s", c->expression->toString()); builder->target->emitTraceMessage(builder, msgStr.c_str()); } else { - msgStr = Util::printf_format("Control: key %s=0x%%llx", c->expression->toString()); - varStr = Util::printf_format("(unsigned long long) %s.%s", keyName.c_str(), - fieldName.c_str()); + msgStr = absl::StrFormat("Control: key %s=0x%%llx", c->expression->toString()); + varStr = + absl::StrFormat("(unsigned long long) %s.%s", keyName.c_str(), fieldName.c_str()); builder->target->emitTraceMessage(builder, msgStr.c_str(), 1, varStr.c_str()); } } @@ -538,21 +536,20 @@ void EBPFTable::emitAction(CodeBuilder *builder, cstring valueName, cstring acti builder->newline(); builder->increaseIndent(); - msgStr = Util::printf_format("Control: executing action %s", name); + msgStr = absl::StrFormat("Control: executing action %s", name); builder->target->emitTraceMessage(builder, msgStr.c_str()); for (auto param : *(action->parameters)) { auto etype = EBPFTypeFactory::instance->create(param->type); unsigned width = etype->as().widthInBits(); if (width <= 64) { - convStr = Util::printf_format("(unsigned long long) (%s->u.%s.%s)", valueName, name, - param->toString()); - msgStr = Util::printf_format("Control: param %s=0x%%llx (%d bits)", - param->toString(), width); + convStr = absl::StrFormat("(unsigned long long) (%s->u.%s.%s)", valueName, name, + param->toString()); + msgStr = absl::StrFormat("Control: param %s=0x%%llx (%d bits)", param->toString(), + width); builder->target->emitTraceMessage(builder, msgStr.c_str(), 1, convStr.c_str()); } else { - msgStr = - Util::printf_format("Control: param %s (%d bits)", param->toString(), width); + msgStr = absl::StrFormat("Control: param %s (%d bits)", param->toString(), width); builder->target->emitTraceMessage(builder, msgStr.c_str()); } } @@ -782,7 +779,7 @@ void EBPFTable::emitLookup(CodeBuilder *builder, cstring key, cstring value) { builder->emitIndent(); builder->appendFormat("for (int i = 0; i < sizeof(struct %s_mask) / 4; i++) ", keyTypeName); builder->blockStart(); - cstring str = Util::printf_format("*(((__u32 *) &%s) + i)", key); + cstring str = absl::StrFormat("*(((__u32 *) &%s) + i)", key); builder->target->emitTraceMessage( builder, "Control: [Ternary] Masking next 4 bytes of %llx with mask %llx", 2, str, "mask[i]"); @@ -806,8 +803,9 @@ void EBPFTable::emitLookup(CodeBuilder *builder, cstring key, cstring value) { builder->append("if (!tuple) "); builder->blockStart(); builder->target->emitTraceMessage( - builder, Util::printf_format("Control: Tuples map %s not found during ternary lookup. Bug?", - instanceName)); + builder, absl::StrFormat("Control: Tuples map %s not found during ternary lookup. Bug?", + instanceName) + .c_str()); builder->emitIndent(); builder->append("break;"); builder->newline(); @@ -861,7 +859,7 @@ cstring EBPFTable::p4ActionToActionIDName(const IR::P4Action *action) const { cstring actionName = EBPFObject::externalName(action); cstring tableInstance = dataMapName; - return Util::printf_format("%s_ACT_%s", tableInstance.toUpper(), actionName.toUpper()); + return absl::StrFormat("%s_ACT_%s", tableInstance.toUpper(), actionName.toUpper()); } /// As ternary has precedence over lpm, this function checks if any @@ -1145,7 +1143,7 @@ void EBPFValueSet::emitTypes(CodeBuilder *builder) { } else if (auto tuple = elemType->to()) { int i = 0; for (auto field : tuple->components) { - cstring name = Util::printf_format("field%d", i++); + cstring name = absl::StrFormat("field%d", i++); fieldEmitter(field, name); fieldNames.emplace_back(std::make_pair(name, field)); } @@ -1191,7 +1189,7 @@ void EBPFValueSet::emitKeyInitializer(CodeBuilder *builder, const IR::SelectExpr } cstring dst = - Util::printf_format("%s.%s", keyVarName.c_str(), fieldNames.at(i).first.c_str()); + absl::StrFormat("%s.%s", keyVarName.c_str(), fieldNames.at(i).first.c_str()); builder->appendFormat("__builtin_memcpy(&%s, &(", dst.c_str()); codeGen->visit(keyExpr); builder->appendFormat("[0]), sizeof(%s))", dst.c_str()); diff --git a/backends/ebpf/psa/ebpfPipeline.cpp b/backends/ebpf/psa/ebpfPipeline.cpp index f66546875b..fee84ee04f 100644 --- a/backends/ebpf/psa/ebpfPipeline.cpp +++ b/backends/ebpf/psa/ebpfPipeline.cpp @@ -255,11 +255,11 @@ void EBPFIngressPipeline::emit(CodeBuilder *builder) { emitMetadataFromCPUMAP(builder); builder->newline(); - msgStr = Util::printf_format( + msgStr = absl::StrFormat( "%s parser: parsing new packet, input_port=%%d, path=%%d, " "pkt_len=%%d", sectionName); - varStr = Util::printf_format("%s->packet_path", compilerGlobalMetadata); + varStr = absl::StrFormat("%s->packet_path", compilerGlobalMetadata); builder->target->emitTraceMessage(builder, msgStr.c_str(), 3, inputPortVar.c_str(), varStr, lengthVar.c_str()); @@ -274,20 +274,20 @@ void EBPFIngressPipeline::emit(CodeBuilder *builder) { builder->spc(); builder->blockStart(); emitPSAControlInputMetadata(builder); - msgStr = Util::printf_format("%s control: packet processing started", sectionName); + msgStr = absl::StrFormat("%s control: packet processing started", sectionName); builder->target->emitTraceMessage(builder, msgStr.c_str()); control->emit(builder); builder->blockEnd(true); - msgStr = Util::printf_format("%s control: packet processing finished", sectionName); + msgStr = absl::StrFormat("%s control: packet processing finished", sectionName); builder->target->emitTraceMessage(builder, msgStr.c_str()); // DEPARSER builder->emitIndent(); builder->blockStart(); - msgStr = Util::printf_format("%s deparser: packet deparsing started", sectionName); + msgStr = absl::StrFormat("%s deparser: packet deparsing started", sectionName); builder->target->emitTraceMessage(builder, msgStr.c_str()); deparser->emit(builder); - msgStr = Util::printf_format("%s deparser: packet deparsing finished", sectionName); + msgStr = absl::StrFormat("%s deparser: packet deparsing finished", sectionName); builder->target->emitTraceMessage(builder, msgStr.c_str()); builder->blockEnd(true); @@ -425,11 +425,11 @@ void EBPFEgressPipeline::emit(CodeBuilder *builder) { emitPSAControlOutputMetadata(builder); emitPSAControlInputMetadata(builder); - msgStr = Util::printf_format( + msgStr = absl::StrFormat( "%s parser: parsing new packet, input_port=%%d, path=%%d, " "pkt_len=%%d", sectionName); - varStr = Util::printf_format("%s->packet_path", compilerGlobalMetadata); + varStr = absl::StrFormat("%s->packet_path", compilerGlobalMetadata); builder->target->emitTraceMessage(builder, msgStr.c_str(), 3, inputPortVar.c_str(), varStr, lengthVar.c_str()); @@ -449,20 +449,20 @@ void EBPFEgressPipeline::emit(CodeBuilder *builder) { builder->emitIndent(); builder->blockStart(); builder->newline(); - msgStr = Util::printf_format("%s control: packet processing started", sectionName); + msgStr = absl::StrFormat("%s control: packet processing started", sectionName); builder->target->emitTraceMessage(builder, msgStr.c_str()); control->emit(builder); builder->blockEnd(true); - msgStr = Util::printf_format("%s control: packet processing finished", sectionName); + msgStr = absl::StrFormat("%s control: packet processing finished", sectionName); builder->target->emitTraceMessage(builder, msgStr.c_str()); // DEPARSER builder->emitIndent(); builder->blockStart(); - msgStr = Util::printf_format("%s deparser: packet deparsing started", sectionName); + msgStr = absl::StrFormat("%s deparser: packet deparsing started", sectionName); builder->target->emitTraceMessage(builder, msgStr.c_str()); deparser->emit(builder); - msgStr = Util::printf_format("%s deparser: packet deparsing finished", sectionName); + msgStr = absl::StrFormat("%s deparser: packet deparsing finished", sectionName); builder->target->emitTraceMessage(builder, msgStr.c_str()); builder->blockEnd(true); @@ -557,7 +557,7 @@ void TCIngressPipeline::emitTCWorkaroundUsingCPUMAP(CodeBuilder *builder) { /// - send to port void TCIngressPipeline::emitTrafficManager(CodeBuilder *builder) { cstring mcast_grp = - Util::printf_format("%s.multicast_group", control->outputStandardMetadata->name.name); + absl::StrFormat("%s.multicast_group", control->outputStandardMetadata->name.name); builder->emitIndent(); builder->appendFormat("if (%s != 0) ", mcast_grp.c_str()); builder->blockStart(); @@ -600,10 +600,9 @@ void TCIngressPipeline::emitTrafficManager(CodeBuilder *builder) { builder->endOfStatement(true); builder->blockEnd(true); - cstring eg_port = - Util::printf_format("%s.egress_port", control->outputStandardMetadata->name.name); + cstring eg_port = absl::StrFormat("%s.egress_port", control->outputStandardMetadata->name.name); cstring cos = - Util::printf_format("%s.class_of_service", control->outputStandardMetadata->name.name); + absl::StrFormat("%s.class_of_service", control->outputStandardMetadata->name.name); builder->target->emitTraceMessage( builder, "IngressTM: Sending packet out of port %d with priority %d", 2, eg_port, cos); builder->emitIndent(); @@ -655,15 +654,14 @@ void TCEgressPipeline::emitTrafficManager(CodeBuilder *builder) { builder->appendFormat("%s->packet_path = RECIRCULATE", compilerGlobalMetadata); builder->endOfStatement(true); builder->emitIndent(); - builder->appendFormat("return bpf_redirect(PSA_PORT_RECIRCULATE, BPF_F_INGRESS)", - contextVar.c_str()); + builder->appendFormat("return bpf_redirect(PSA_PORT_RECIRCULATE, BPF_F_INGRESS)"); builder->endOfStatement(true); builder->blockEnd(true); builder->newline(); // normal packet to port - varStr = Util::printf_format("%s->ifindex", contextVar); + varStr = absl::StrFormat("%s->ifindex", contextVar); builder->target->emitTraceMessage(builder, "EgressTM: output packet to port %d", 1, varStr.c_str()); builder->emitIndent(); @@ -699,8 +697,7 @@ void XDPIngressPipeline::emitGlobalMetadataInitializer(CodeBuilder *builder) { void XDPIngressPipeline::emitTrafficManager(CodeBuilder *builder) { // do not handle multicast; it has been handled earlier by PreDeparser. - cstring portVar = - Util::printf_format("%s.egress_port", control->outputStandardMetadata->name.name); + cstring portVar = absl::StrFormat("%s.egress_port", control->outputStandardMetadata->name.name); builder->target->emitTraceMessage(builder, "IngressTM: Sending packet out of port %u", 1, portVar); builder->emitIndent(); @@ -739,7 +736,7 @@ void XDPEgressPipeline::emitTrafficManager(CodeBuilder *builder) { builder->newline(); // normal packet to port - varStr = Util::printf_format("%s.egress_port", control->inputStandardMetadata->name.name); + varStr = absl::StrFormat("%s.egress_port", control->inputStandardMetadata->name.name); builder->target->emitTraceMessage(builder, "EgressTM: output packet to port %d", 1, varStr); builder->emitIndent(); builder->appendFormat("return %s;", this->forwardReturnCode()); @@ -785,11 +782,11 @@ void TCTrafficManagerForXDP::emit(CodeBuilder *builder) { emitReadXDP2TCMetadataFromHead(builder); } - msgStr = Util::printf_format("%s deparser: packet deparsing started", sectionName); + msgStr = absl::StrFormat("%s deparser: packet deparsing started", sectionName); builder->target->emitTraceMessage(builder, msgStr.c_str()); builder->emitIndent(); deparser->emit(builder); - msgStr = Util::printf_format("%s deparser: packet deparsing finished", sectionName); + msgStr = absl::StrFormat("%s deparser: packet deparsing finished", sectionName); builder->target->emitTraceMessage(builder, msgStr.c_str()); this->emitTrafficManager(builder); diff --git a/backends/ebpf/psa/ebpfPsaControl.cpp b/backends/ebpf/psa/ebpfPsaControl.cpp index 79411b12bc..00aa9c5577 100644 --- a/backends/ebpf/psa/ebpfPsaControl.cpp +++ b/backends/ebpf/psa/ebpfPsaControl.cpp @@ -48,7 +48,7 @@ bool ControlBodyTranslatorPSA::preorder(const IR::AssignmentStatement *a) { ext->originalExternType->name.name == "DirectMeter") { // It is just for trace message before meter execution cstring name = EBPFObject::externalName(ext->object); - auto msgStr = Util::printf_format("Executing meter: %s", name); + auto msgStr = absl::StrFormat("Executing meter: %s", name); builder->target->emitTraceMessage(builder, msgStr.c_str()); } } diff --git a/backends/ebpf/psa/ebpfPsaDeparser.cpp b/backends/ebpf/psa/ebpfPsaDeparser.cpp index 4c6821e667..c66fdb0ddb 100644 --- a/backends/ebpf/psa/ebpfPsaDeparser.cpp +++ b/backends/ebpf/psa/ebpfPsaDeparser.cpp @@ -263,7 +263,7 @@ void XDPIngressDeparserPSA::emitPreDeparser(CodeBuilder *builder) { builder->endOfStatement(true); builder->blockEnd(true); builder->emitIndent(); - builder->appendFormat("if (%s->drop) ", istd->name.name, istd->name.name); + builder->appendFormat("if (%s->drop) ", istd->name.name); builder->blockStart(); builder->target->emitTraceMessage(builder, "PreDeparser: dropping packet.."); builder->emitIndent(); diff --git a/backends/ebpf/psa/ebpfPsaGen.h b/backends/ebpf/psa/ebpfPsaGen.h index e3159000fe..5979227b8f 100644 --- a/backends/ebpf/psa/ebpfPsaGen.h +++ b/backends/ebpf/psa/ebpfPsaGen.h @@ -47,8 +47,8 @@ class EbpfCodeGenerator { class PSAEbpfGenerator : public EbpfCodeGenerator { public: - static const unsigned MaxClones = 64; - static const unsigned MaxCloneSessions = 1024; + static constexpr unsigned MaxClones = 64; + static constexpr unsigned MaxCloneSessions = 1024; EBPFPipeline *ingress; EBPFPipeline *egress; @@ -98,7 +98,7 @@ class PSAArchXDP : public PSAEbpfGenerator { EBPFPipeline *tcIngressForXDP; /// If the XDP mode is used, we need to have TC Egress pipeline to handle cloned packets. EBPFPipeline *tcEgressForXDP; - static const unsigned egressDevmapSize = 256; + static constexpr unsigned egressDevmapSize = 256; PSAArchXDP(const EbpfOptions &options, std::vector &ebpfTypes, EBPFPipeline *xdpIngress, EBPFPipeline *xdpEgress, EBPFPipeline *tcTrafficManager, diff --git a/backends/ebpf/psa/ebpfPsaTable.cpp b/backends/ebpf/psa/ebpfPsaTable.cpp index 2a53fa8824..4f66a0e40e 100644 --- a/backends/ebpf/psa/ebpfPsaTable.cpp +++ b/backends/ebpf/psa/ebpfPsaTable.cpp @@ -634,16 +634,15 @@ void EBPFTablePSA::emitMapUpdateTraceMsg(CodeBuilder *builder, cstring mapName, builder->emitIndent(); builder->appendFormat("if (%s) ", returnCode.c_str()); builder->blockStart(); - cstring msgStr = Util::printf_format("Map initializer: Error while map (%s) update, code: %s", - mapName, "%d"); + cstring msgStr = + absl::StrFormat("Map initializer: Error while map (%s) update, code: %s", mapName, "%d"); builder->target->emitTraceMessage(builder, msgStr, 1, returnCode.c_str()); builder->blockEnd(false); builder->append(" else "); builder->blockStart(); - msgStr = Util::printf_format("Map initializer: Map (%s) update succeed", mapName, - returnCode.c_str()); + msgStr = absl::StrFormat("Map initializer: Map (%s) update succeed", mapName); builder->target->emitTraceMessage(builder, msgStr); builder->blockEnd(true); } diff --git a/backends/ebpf/psa/externs/ebpfPsaCounter.cpp b/backends/ebpf/psa/externs/ebpfPsaCounter.cpp index 145c9459d1..b0b37351b3 100644 --- a/backends/ebpf/psa/externs/ebpfPsaCounter.cpp +++ b/backends/ebpf/psa/externs/ebpfPsaCounter.cpp @@ -200,13 +200,13 @@ void EBPFCounterPSA::emitDirectMethodInvocation(CodeBuilder *builder, CHECK_NULL(pipeline); cstring msgStr = - Util::printf_format("Counter: updating %s, packets=1, bytes=%%u", instanceName.c_str()); - cstring varStr = Util::printf_format("%s", pipeline->lengthVar.c_str()); + absl::StrFormat("Counter: updating %s, packets=1, bytes=%%u", instanceName.c_str()); + cstring varStr = absl::StrFormat("%s", pipeline->lengthVar.c_str()); builder->target->emitTraceMessage(builder, msgStr.c_str(), 1, varStr.c_str()); emitCounterUpdate(builder, target, ""_cs); - msgStr = Util::printf_format("Counter: %s updated", instanceName.c_str()); + msgStr = absl::StrFormat("Counter: %s updated", instanceName.c_str()); builder->target->emitTraceMessage(builder, msgStr.c_str()); } @@ -231,9 +231,9 @@ void EBPFCounterPSA::emitCount(CodeBuilder *builder, const IR::MethodCallExpress codeGen->visit(index); builder->endOfStatement(true); - msgStr = Util::printf_format("Counter: updating %s, id=%%u, packets=1, bytes=%%u", - instanceName.c_str()); - varStr = Util::printf_format("%s", program->lengthVar.c_str()); + msgStr = + absl::StrFormat("Counter: updating %s, id=%%u, packets=1, bytes=%%u", instanceName.c_str()); + varStr = absl::StrFormat("%s", program->lengthVar.c_str()); builder->target->emitTraceMessage(builder, msgStr.c_str(), 2, keyName.c_str(), varStr.c_str()); builder->emitIndent(); @@ -242,7 +242,7 @@ void EBPFCounterPSA::emitCount(CodeBuilder *builder, const IR::MethodCallExpress emitCounterUpdate(builder, valueName, keyName); - msgStr = Util::printf_format("Counter: %s updated", instanceName.c_str()); + msgStr = absl::StrFormat("Counter: %s updated", instanceName.c_str()); builder->target->emitTraceMessage(builder, msgStr.c_str()); } @@ -263,7 +263,7 @@ void EBPFCounterPSA::emitCounterUpdate(CodeBuilder *builder, const cstring targe program->lengthVar); builder->endOfStatement(true); - varStr = Util::printf_format("%sbytes", targetWAccess.c_str()); + varStr = absl::StrFormat("%sbytes", targetWAccess.c_str()); builder->target->emitTraceMessage(builder, "Counter: now bytes=%u", 1, varStr.c_str()); } if (type == CounterType::PACKETS || type == CounterType::PACKETS_AND_BYTES) { @@ -271,7 +271,7 @@ void EBPFCounterPSA::emitCounterUpdate(CodeBuilder *builder, const cstring targe builder->appendFormat("__sync_fetch_and_add(&(%spackets), 1)", targetWAccess.c_str()); builder->endOfStatement(true); - varStr = Util::printf_format("%spackets", targetWAccess.c_str()); + varStr = absl::StrFormat("%spackets", targetWAccess.c_str()); builder->target->emitTraceMessage(builder, "Counter: now packets=%u", 1, varStr.c_str()); } diff --git a/backends/ebpf/psa/externs/ebpfPsaDigest.cpp b/backends/ebpf/psa/externs/ebpfPsaDigest.cpp index 11f91cbc93..07d2888f07 100644 --- a/backends/ebpf/psa/externs/ebpfPsaDigest.cpp +++ b/backends/ebpf/psa/externs/ebpfPsaDigest.cpp @@ -82,7 +82,7 @@ class EBPFDigestPSAValueVisitor : public CodeGenInspector { for (const auto *f : se->components) { auto type = typeMap->getType(f->expression); - cstring path = Util::printf_format("%s.%s", tmpVar, f->name.name); + cstring path = absl::StrFormat("%s.%s", tmpVar, f->name.name); codegen->emitAssignStatement(type, nullptr, path, f->expression); builder->newline(); } diff --git a/backends/ebpf/psa/externs/ebpfPsaHashAlgorithm.cpp b/backends/ebpf/psa/externs/ebpfPsaHashAlgorithm.cpp index 653b6a4a8e..ef50b361ac 100644 --- a/backends/ebpf/psa/externs/ebpfPsaHashAlgorithm.cpp +++ b/backends/ebpf/psa/externs/ebpfPsaHashAlgorithm.cpp @@ -296,11 +296,11 @@ void CRCChecksumAlgorithm::emitAddData(CodeBuilder *builder, const ArgumentsList } } - cstring varStr = Util::printf_format("(u64) %s", registerVar.c_str()); + cstring varStr = absl::StrFormat("(u64) %s", registerVar.c_str()); builder->target->emitTraceMessage(builder, "CRC: checksum state: %llx", 1, varStr.c_str()); cstring final_crc = - Util::printf_format("(u64) %s(%s)", finalizeMethod.c_str(), registerVar.c_str()); + absl::StrFormat("(u64) %s(%s)", finalizeMethod.c_str(), registerVar.c_str()); builder->target->emitTraceMessage(builder, "CRC: final checksum: %llx", 1, final_crc.c_str()); builder->blockEnd(true); diff --git a/backends/ebpf/psa/externs/ebpfPsaRegister.cpp b/backends/ebpf/psa/externs/ebpfPsaRegister.cpp index e065e05e27..697324146a 100644 --- a/backends/ebpf/psa/externs/ebpfPsaRegister.cpp +++ b/backends/ebpf/psa/externs/ebpfPsaRegister.cpp @@ -135,8 +135,8 @@ void EBPFRegisterPSA::emitInitializer(CodeBuilder *builder) { builder->emitIndent(); builder->appendFormat("if (%s) ", ret.c_str()); builder->blockStart(); - cstring msgStr = Util::printf_format("Map initializer: Error while map (%s) update, code: %s", - instanceName, "%d"); + cstring msgStr = absl::StrFormat("Map initializer: Error while map (%s) update, code: %s", + instanceName, "%d"); builder->target->emitTraceMessage(builder, msgStr, 1, ret.c_str()); builder->blockEnd(true); @@ -160,7 +160,7 @@ void EBPFRegisterPSA::emitRegisterRead(CodeBuilder *builder, const P4::ExternMet this->valueType->declare(builder, valueName, true); builder->endOfStatement(true); - cstring msgStr = Util::printf_format("Register: reading %s", instanceName.c_str()); + cstring msgStr = absl::StrFormat("Register: reading %s", instanceName.c_str()); builder->target->emitTraceMessage(builder, msgStr.c_str()); builder->emitIndent(); @@ -215,7 +215,7 @@ void EBPFRegisterPSA::emitRegisterRead(CodeBuilder *builder, const P4::ExternMet void EBPFRegisterPSA::emitRegisterWrite(CodeBuilder *builder, const P4::ExternMethod *method, ControlBodyTranslatorPSA *translator) { - cstring msgStr = Util::printf_format("Register: writing %s", instanceName.c_str()); + cstring msgStr = absl::StrFormat("Register: writing %s", instanceName.c_str()); builder->target->emitTraceMessage(builder, msgStr.c_str()); builder->emitIndent(); @@ -230,8 +230,7 @@ void EBPFRegisterPSA::emitRegisterWrite(CodeBuilder *builder, const P4::ExternMe builder->emitIndent(); builder->appendFormat("if (%s) ", ret.c_str()); builder->blockStart(); - msgStr = - Util::printf_format("Register: Error while map (%s) update, code: %s", instanceName, "%d"); + msgStr = absl::StrFormat("Register: Error while map (%s) update, code: %s", instanceName, "%d"); builder->target->emitTraceMessage(builder, msgStr, 1, ret.c_str()); builder->blockEnd(true); diff --git a/backends/ebpf/psa/externs/ebpfPsaTableImplementation.cpp b/backends/ebpf/psa/externs/ebpfPsaTableImplementation.cpp index 1383e54638..ff906747f3 100644 --- a/backends/ebpf/psa/externs/ebpfPsaTableImplementation.cpp +++ b/backends/ebpf/psa/externs/ebpfPsaTableImplementation.cpp @@ -165,12 +165,11 @@ void EBPFActionProfilePSA::emitInstance(CodeBuilder *builder) { void EBPFActionProfilePSA::applyImplementation(CodeBuilder *builder, cstring tableValueName, cstring actionRunVariable) { - cstring msg = Util::printf_format("ActionProfile: applying %s", instanceName.c_str()); + cstring msg = absl::StrFormat("ActionProfile: applying %s", instanceName.c_str()); builder->target->emitTraceMessage(builder, msg.c_str()); cstring apValueName = program->refMap->newName("ap_value"); - cstring apKeyName = - Util::printf_format("%s->%s", tableValueName.c_str(), referenceName.c_str()); + cstring apKeyName = absl::StrFormat("%s->%s", tableValueName.c_str(), referenceName.c_str()); builder->target->emitTraceMessage(builder, "ActionProfile: entry id %u", 1, apKeyName.c_str()); @@ -200,7 +199,7 @@ void EBPFActionProfilePSA::applyImplementation(CodeBuilder *builder, cstring tab builder->endOfStatement(true); builder->blockEnd(true); - msg = Util::printf_format("ActionProfile: %s applied", instanceName.c_str()); + msg = absl::StrFormat("ActionProfile: %s applied", instanceName.c_str()); builder->target->emitTraceMessage(builder, msg.c_str()); } @@ -236,7 +235,7 @@ EBPFActionSelectorPSA::EBPFActionSelectorPSA(const EBPFProgram *program, CodeGen "value truncation, must be at least 1 bit", decl->arguments->at(2)->expression); } - outputHashMask = Util::printf_format("0x%llx", (1ull << outputHashWidth) - 1); + outputHashMask = absl::StrFormat("0x%llx", (1ull << outputHashWidth) - 1); // map names actionsMapName = instanceName + "_actions"; @@ -330,7 +329,7 @@ void EBPFActionSelectorPSA::applyImplementation(CodeBuilder *builder, cstring ta cstring actionRunVariable) { if (hashEngine == nullptr) return; - cstring msg = Util::printf_format("ActionSelector: applying %s", instanceName.c_str()); + cstring msg = absl::StrFormat("ActionSelector: applying %s", instanceName.c_str()); builder->target->emitTraceMessage(builder, msg.c_str()); // 1. Declare variables. @@ -518,7 +517,7 @@ void EBPFActionSelectorPSA::applyImplementation(CodeBuilder *builder, cstring ta } builder->blockEnd(true); - msg = Util::printf_format("ActionSelector: %s applied", instanceName.c_str()); + msg = absl::StrFormat("ActionSelector: %s applied", instanceName.c_str()); builder->target->emitTraceMessage(builder, msg.c_str()); } @@ -674,7 +673,7 @@ void EBPFActionSelectorPSA::emitCacheTypes(CodeBuilder *builder) { for (auto s : selectors) { auto type = program->typeMap->getType(s->expression); auto ebpfType = EBPFTypeFactory::instance->create(type); - cstring fieldName = Util::printf_format("field%u", fieldNumber++); + cstring fieldName = absl::StrFormat("field%u", fieldNumber++); builder->emitIndent(); ebpfType->declare(builder, fieldName, false); @@ -719,7 +718,7 @@ void EBPFActionSelectorPSA::emitCacheLookup(CodeBuilder *builder, cstring key, c for (auto s : selectors) { auto type = program->typeMap->getType(s->expression); auto ebpfType = EBPFTypeFactory::instance->create(type); - cstring fieldName = Util::printf_format("field%u", fieldNumber++); + cstring fieldName = absl::StrFormat("field%u", fieldNumber++); bool memcpy = false; auto scalar = ebpfType->to(); diff --git a/backends/ebpf/target.cpp b/backends/ebpf/target.cpp index a00b35f38c..78e78e1f90 100644 --- a/backends/ebpf/target.cpp +++ b/backends/ebpf/target.cpp @@ -67,8 +67,8 @@ void KernelSamplesTarget::emitTableDecl(Util::SourceCodeBuilder *builder, cstrin TableKind tableKind, cstring keyType, cstring valueType, unsigned size) const { cstring kind, flags; - cstring registerTable = "REGISTER_TABLE(%s, %s, %s, %s, %d)"_cs; - cstring registerTableWithFlags = "REGISTER_TABLE_FLAGS(%s, %s, %s, %s, %d, %s)"_cs; + static constexpr char registerTable[] = "REGISTER_TABLE(%s, %s, %s, %s, %d)"; + static constexpr char registerTableWithFlags[] = "REGISTER_TABLE_FLAGS(%s, %s, %s, %s, %d, %s)"; kind = getBPFMapType(tableKind); @@ -121,8 +121,9 @@ void KernelSamplesTarget::emitMapInMapDecl(Util::SourceCodeBuilder *builder, cst BUG("Unsupported type of outer map for map-in-map"); } - cstring registerOuterTable = "REGISTER_TABLE_OUTER(%s, %s_OF_MAPS, %s, %s, %d, %d, %s)"_cs; - cstring registerInnerTable = "REGISTER_TABLE_INNER(%s, %s, %s, %s, %d, %d, %d)"_cs; + static constexpr char registerOuterTable[] = + "REGISTER_TABLE_OUTER(%s, %s_OF_MAPS, %s, %s, %d, %d, %s)"; + static constexpr char registerInnerTable[] = "REGISTER_TABLE_INNER(%s, %s, %s, %s, %d, %d, %d)"; innerMapIndex++; diff --git a/backends/tc/ebpfCodeGen.cpp b/backends/tc/ebpfCodeGen.cpp index c6459cb15c..ae0a3118f5 100644 --- a/backends/tc/ebpfCodeGen.cpp +++ b/backends/tc/ebpfCodeGen.cpp @@ -315,11 +315,11 @@ void TCIngressPipelinePNA::emit(EBPF::CodeBuilder *builder) { builder->newline(); if (name == "tc-parse") { - msgStr = Util::printf_format( + msgStr = absl::StrFormat( "%s parser: parsing new packet, input_port=%%d, path=%%d, " "pkt_len=%%d", sectionName); - varStr = Util::printf_format("%s->packet_path", compilerGlobalMetadata); + varStr = absl::StrFormat("%s->packet_path", compilerGlobalMetadata); builder->target->emitTraceMessage(builder, msgStr.c_str(), 3, inputPortVar.c_str(), varStr, lengthVar.c_str()); @@ -336,21 +336,21 @@ void TCIngressPipelinePNA::emit(EBPF::CodeBuilder *builder) { if (name == "tc-ingress") { // CONTROL builder->blockStart(); - msgStr = Util::printf_format("%s control: packet processing started", sectionName); + msgStr = absl::StrFormat("%s control: packet processing started", sectionName); builder->target->emitTraceMessage(builder, msgStr.c_str()); ((EBPFControlPNA *)control)->emitExternDefinition(builder); control->emit(builder); builder->blockEnd(true); - msgStr = Util::printf_format("%s control: packet processing finished", sectionName); + msgStr = absl::StrFormat("%s control: packet processing finished", sectionName); builder->target->emitTraceMessage(builder, msgStr.c_str()); // DEPARSER builder->emitIndent(); builder->blockStart(); - msgStr = Util::printf_format("%s deparser: packet deparsing started", sectionName); + msgStr = absl::StrFormat("%s deparser: packet deparsing started", sectionName); builder->target->emitTraceMessage(builder, msgStr.c_str()); deparser->emit(builder); - msgStr = Util::printf_format("%s deparser: packet deparsing finished", sectionName); + msgStr = absl::StrFormat("%s deparser: packet deparsing finished", sectionName); builder->target->emitTraceMessage(builder, msgStr.c_str()); builder->blockEnd(true); } @@ -505,9 +505,9 @@ void TCIngressPipelinePNA::emitTrafficManager(EBPF::CodeBuilder *builder) { builder->endOfStatement(true); builder->blockEnd(true); - cstring eg_port = Util::printf_format("%s->egress_port", compilerGlobalMetadata); + cstring eg_port = absl::StrFormat("%s->egress_port", compilerGlobalMetadata); cstring cos = - Util::printf_format("%s.class_of_service", control->outputStandardMetadata->name.name); + absl::StrFormat("%s.class_of_service", control->outputStandardMetadata->name.name); builder->target->emitTraceMessage( builder, "IngressTM: Sending packet out of port %d with priority %d", 2, eg_port, cos); builder->emitIndent(); @@ -625,7 +625,7 @@ void PnaStateTranslationVisitor::compileExtractField(const IR::Expression *expr, } } - msgStr = Util::printf_format("Parser: extracting field %s", fieldName); + msgStr = absl::StrFormat("Parser: extracting field %s", fieldName); builder->target->emitTraceMessage(builder, msgStr.c_str()); if (widthToExtract <= 64) { @@ -741,13 +741,13 @@ void PnaStateTranslationVisitor::compileExtractField(const IR::Expression *expr, } } } - cstring tmp = Util::printf_format("(unsigned long long) %s.%s", exprStr, fieldName); + cstring tmp = absl::StrFormat("(unsigned long long) %s.%s", exprStr, fieldName); - msgStr = Util::printf_format("Parser: extracted %s=0x%%llx (%u bits)", fieldName, - widthToExtract); + msgStr = + absl::StrFormat("Parser: extracted %s=0x%%llx (%u bits)", fieldName, widthToExtract); builder->target->emitTraceMessage(builder, msgStr.c_str(), 1, tmp.c_str()); } else { - msgStr = Util::printf_format("Parser: extracted %s (%u bits)", fieldName, widthToExtract); + msgStr = absl::StrFormat("Parser: extracted %s (%u bits)", fieldName, widthToExtract); builder->target->emitTraceMessage(builder, msgStr.c_str()); } @@ -755,9 +755,9 @@ void PnaStateTranslationVisitor::compileExtractField(const IR::Expression *expr, } void PnaStateTranslationVisitor::compileLookahead(const IR::Expression *destination) { - cstring msgStr = Util::printf_format("Parser: lookahead for %s %s", - state->parser->typeMap->getType(destination)->toString(), - destination->toString()); + cstring msgStr = absl::StrFormat("Parser: lookahead for %s %s", + state->parser->typeMap->getType(destination)->toString(), + destination->toString()); builder->target->emitTraceMessage(builder, msgStr.c_str()); builder->emitIndent(); @@ -916,7 +916,7 @@ void EBPFTablePNA::emitValueStructStructure(EBPF::CodeBuilder *builder) { cstring EBPFTablePNA::p4ActionToActionIDName(const IR::P4Action *action) const { cstring actionName = EBPFObject::externalName(action); cstring tableInstance = dataMapName; - return Util::printf_format("%s_ACT_%s", tableInstance.toUpper(), actionName.toUpper()); + return absl::StrFormat("%s_ACT_%s", tableInstance.toUpper(), actionName.toUpper()); } void EBPFTablePNA::emitActionArguments(EBPF::CodeBuilder *builder, const IR::P4Action *action, @@ -963,21 +963,20 @@ void EBPFTablePNA::emitAction(EBPF::CodeBuilder *builder, cstring valueName, builder->newline(); builder->increaseIndent(); - msgStr = Util::printf_format("Control: executing action %s", name); + msgStr = absl::StrFormat("Control: executing action %s", name); builder->target->emitTraceMessage(builder, msgStr.c_str()); for (auto param : *(action->parameters)) { auto etype = EBPF::EBPFTypeFactory::instance->create(param->type); unsigned width = etype->as().widthInBits(); if (width <= 64) { - convStr = Util::printf_format("(unsigned long long) (%s->u.%s.%s)", valueName, name, - param->toString()); - msgStr = Util::printf_format("Control: param %s=0x%%llx (%d bits)", - param->toString(), width); + convStr = absl::StrFormat("(unsigned long long) (%s->u.%s.%s)", valueName, name, + param->toString()); + msgStr = absl::StrFormat("Control: param %s=0x%%llx (%d bits)", param->toString(), + width); builder->target->emitTraceMessage(builder, msgStr.c_str(), 1, convStr.c_str()); } else { - msgStr = - Util::printf_format("Control: param %s (%d bits)", param->toString(), width); + msgStr = absl::StrFormat("Control: param %s (%d bits)", param->toString(), width); builder->target->emitTraceMessage(builder, msgStr.c_str()); } } @@ -1058,7 +1057,7 @@ void EBPFTablePNA::emitAction(EBPF::CodeBuilder *builder, cstring valueName, cstring noActionName = P4::P4CoreLibrary::instance().noAction.name; cstring tableInstance = dataMapName; cstring actionName = - Util::printf_format("%s_ACT_%s", tableInstance.toUpper(), noActionName.toUpper()); + absl::StrFormat("%s_ACT_%s", tableInstance.toUpper(), noActionName.toUpper()); builder->emitIndent(); builder->appendFormat("case %s: ", actionName); builder->newline(); @@ -1123,7 +1122,7 @@ void EBPFTablePNA::emitValueActionIDNames(EBPF::CodeBuilder *builder) { cstring noActionName = P4::P4CoreLibrary::instance().noAction.name; cstring tableInstance = dataMapName; cstring actionName = - Util::printf_format("%s_ACT_%s", tableInstance.toUpper(), noActionName.toUpper()); + absl::StrFormat("%s_ACT_%s", tableInstance.toUpper(), noActionName.toUpper()); unsigned int action_idx = tcIR->getActionId(noActionName); builder->emitIndent(); builder->appendFormat("#define %s %d", actionName, action_idx); @@ -1751,7 +1750,7 @@ void ControlBodyTranslatorPNA::processApply(const P4::ApplyMethod *method) { auto table = control->getTable(method->object->getName().name); BUG_CHECK(table != nullptr, "No table for %1%", method->expr); - msgStr = Util::printf_format("Control: applying %s", method->object->getName().name); + msgStr = absl::StrFormat("Control: applying %s", method->object->getName().name); builder->target->emitTraceMessage(builder, msgStr.c_str()); builder->emitIndent(); @@ -1851,12 +1850,12 @@ void ControlBodyTranslatorPNA::processApply(const P4::ApplyMethod *method) { cstring msgStr, varStr; if (memcpy) { - msgStr = Util::printf_format("Control: key %s", c->expression->toString()); + msgStr = absl::StrFormat("Control: key %s", c->expression->toString()); builder->target->emitTraceMessage(builder, msgStr.c_str()); } else { - msgStr = Util::printf_format("Control: key %s=0x%%llx", c->expression->toString()); - varStr = Util::printf_format("(unsigned long long) %s.%s", keyname.c_str(), - fieldName.c_str()); + msgStr = absl::StrFormat("Control: key %s=0x%%llx", c->expression->toString()); + varStr = absl::StrFormat("(unsigned long long) %s.%s", keyname.c_str(), + fieldName.c_str()); builder->target->emitTraceMessage(builder, msgStr.c_str(), 1, varStr.c_str()); } } @@ -1914,7 +1913,7 @@ void ControlBodyTranslatorPNA::processApply(const P4::ApplyMethod *method) { toDereference.clear(); builder->blockEnd(true); - msgStr = Util::printf_format("Control: %s applied", method->object->getName().name); + msgStr = absl::StrFormat("Control: %s applied", method->object->getName().name); builder->target->emitTraceMessage(builder, msgStr.c_str()); } @@ -2003,9 +2002,9 @@ cstring ActionTranslationVisitorPNA::getParamInstanceName(const IR::Expression * if (isDefaultAction) { cstring actionName = action->name.originalName; - auto paramStr = Util::printf_format("p4tc_filter_fields.%s_%s_%s", - table->table->container->name.originalName, actionName, - expression->toString()); + auto paramStr = absl::StrFormat("p4tc_filter_fields.%s_%s_%s", + table->table->container->name.originalName, actionName, + expression->toString()); return paramStr; } else { return EBPF::ActionTranslationVisitor::getParamInstanceName(expression); @@ -2073,7 +2072,7 @@ void DeparserHdrEmitTranslatorPNA::processMethod(const P4::ExternMethod *method) builder->blockStart(); auto program = deparser->program; unsigned width = headerToEmit->width_bits(); - msgStr = Util::printf_format("Deparser: emitting header %s", expr->toString().c_str()); + msgStr = absl::StrFormat("Deparser: emitting header %s", expr->toString().c_str()); builder->target->emitTraceMessage(builder, msgStr.c_str()); builder->emitIndent(); @@ -2149,13 +2148,13 @@ void DeparserHdrEmitTranslatorPNA::emitField(EBPF::CodeBuilder *builder, cstring visit(hdrExpr); builder->appendFormat(".%s", field.c_str()); builder->endOfStatement(true); - msgStr = Util::printf_format("Deparser: emitting field %s=0x%%llx (%u bits)", field, - widthToEmit); + msgStr = absl::StrFormat("Deparser: emitting field %s=0x%%llx (%u bits)", field, + widthToEmit); builder->target->emitTraceMessage(builder, msgStr.c_str(), 1, "tmp"); builder->blockEnd(true); } } else { - msgStr = Util::printf_format("Deparser: emitting field %s (%u bits)", field, widthToEmit); + msgStr = absl::StrFormat("Deparser: emitting field %s (%u bits)", field, widthToEmit); builder->target->emitTraceMessage(builder, msgStr.c_str()); } diff --git a/frontends/p4/toP4/toP4.cpp b/frontends/p4/toP4/toP4.cpp index 96ddcdcb9a..fffd2702b1 100644 --- a/frontends/p4/toP4/toP4.cpp +++ b/frontends/p4/toP4/toP4.cpp @@ -1327,8 +1327,8 @@ bool ToP4::preorder(const IR::Annotations *a) { bool ToP4::preorder(const IR::Annotation *a) { builder.append("@"); builder.append(a->name); - char open = a->structured ? '[' : '('; - char close = a->structured ? ']' : ')'; + const char *open = a->structured ? "[" : "("; + const char *close = a->structured ? "]" : ")"; if (!a->expr.empty()) { builder.append(open); setVecSep(", "); diff --git a/ir/visitor.cpp b/ir/visitor.cpp index 043fcf65be..49a6a5b731 100644 --- a/ir/visitor.cpp +++ b/ir/visitor.cpp @@ -798,7 +798,7 @@ cstring Visitor::demangle(const char *str) { } #else #warning "No name demangling available; class names in logs will be mangled" -cstring Visitor::demangle(const char *str) { return str; } +cstring Visitor::demangle(const char *str) { return cstring(str); } #endif #if HAVE_LIBGC diff --git a/lib/error_reporter.h b/lib/error_reporter.h index 8fe7151a12..2dc61ac041 100644 --- a/lib/error_reporter.h +++ b/lib/error_reporter.h @@ -218,6 +218,8 @@ class ErrorReporter { Util::SourcePosition position = sources->getCurrentPosition(); position--; + // Unfortunately, we cannot go with statically checked format string + // here as it would require some changes to yyerror std::string message; if (!absl::FormatUntyped(&message, absl::UntypedFormatSpec(fmt), {absl::FormatArg(args)...})) { diff --git a/lib/sourceCodeBuilder.h b/lib/sourceCodeBuilder.h index 10f770fc6d..78d12adaef 100644 --- a/lib/sourceCodeBuilder.h +++ b/lib/sourceCodeBuilder.h @@ -19,8 +19,8 @@ limitations under the License. #include -#include - +#include "absl/strings/cord.h" +#include "absl/strings/str_format.h" #include "lib/cstring.h" #include "lib/exceptions.h" #include "lib/stringify.h" @@ -30,8 +30,7 @@ class SourceCodeBuilder { int indentLevel; // current indent level unsigned indentAmount; - std::stringstream buffer; - const std::string nl = "\n"; + absl::Cord buffer; bool endsInSpace; bool supressSemi = false; @@ -44,11 +43,11 @@ class SourceCodeBuilder { if (indentLevel < 0) BUG("Negative indent"); } void newline() { - buffer << nl; + buffer.Append("\n"); endsInSpace = true; } void spc() { - if (!endsInSpace) buffer << " "; + if (!endsInSpace) buffer.Append(" "); endsInSpace = true; } @@ -63,25 +62,25 @@ class SourceCodeBuilder { } void append(const std::string &str) { if (str.empty()) return; - endsInSpace = ::isspace(str.at(str.size() - 1)); - buffer << str; + endsInSpace = ::isspace(str.back()); + buffer.Append(str); } + [[deprecated("use string / char* version instead")]] void append(char c) { - endsInSpace = ::isspace(c); - buffer << c; + std::string str(1, c); + append(str); } void append(const char *str) { if (str == nullptr) BUG("Null argument to append"); if (strlen(str) == 0) return; endsInSpace = ::isspace(str[strlen(str) - 1]); - buffer << str; + buffer.Append(str); } - void appendFormat(const char *format, ...) { - va_list ap; - va_start(ap, format); - cstring str = Util::vprintf_format(format, ap); - va_end(ap); - append(str); + + template + void appendFormat(const absl::FormatSpec &format, Args &&...args) { + // FIXME: Sink directly to cord + append(absl::StrFormat(format, std::forward(args)...)); } void append(unsigned u) { appendFormat("%d", u); } void append(int u) { appendFormat("%d", u); } @@ -100,7 +99,7 @@ class SourceCodeBuilder { } void emitIndent() { - buffer << std::string(indentLevel, ' '); + buffer.Append(std::string(indentLevel, ' ')); if (indentLevel > 0) endsInSpace = true; } @@ -111,7 +110,7 @@ class SourceCodeBuilder { if (nl) newline(); } - std::string toString() const { return buffer.str(); } + std::string toString() const { return std::string(buffer); } void commentStart() { append("/* "); } void commentEnd() { append(" */"); } bool lastIsSpace() const { return endsInSpace; } diff --git a/lib/source_file.cpp b/lib/source_file.cpp index f23c877212..31723e8279 100644 --- a/lib/source_file.cpp +++ b/lib/source_file.cpp @@ -19,7 +19,11 @@ limitations under the License. #include #include -#include "exceptions.h" +#include "absl/strings/match.h" +#include "absl/strings/str_cat.h" +#include "absl/strings/str_format.h" +#include "absl/strings/str_replace.h" +#include "lib/exceptions.h" #include "lib/log.h" #include "lib/stringify.h" @@ -35,7 +39,7 @@ SourcePosition::SourcePosition(unsigned lineNumber, unsigned columnNumber) } cstring SourcePosition::toString() const { - return Util::printf_format("%d:%d", lineNumber, columnNumber); + return absl::StrFormat("%d:%d", lineNumber, columnNumber); } ////////////////////////////////////////////////////////////////////////////////////////// @@ -50,7 +54,8 @@ SourceInfo::SourceInfo(const InputSources *sources, SourcePosition start, Source } cstring SourceInfo::toString() const { - return Util::printf_format("(%s)-(%s)", start.toString(), end.toString()); + return absl::StrFormat("(%s)-(%s)", start.toString().string_view(), + end.toString().string_view()); } ////////////////////////////////////////////////////////////////////////////////////////// @@ -64,8 +69,7 @@ void InputSources::addComment(SourceInfo srcInfo, bool singleLine, cstring body) if (!singleLine) // Drop the "*/" body = body.exceptLast(2); - auto comment = new Comment(srcInfo, singleLine, body); - comments.push_back(comment); + comments.push_back(new Comment(srcInfo, singleLine, body)); } /// prevent further changes @@ -93,13 +97,13 @@ void InputSources::appendToLastLine(std::string_view text) { char c = text[i]; if (c == '\n') BUG("Text contains newlines"); } - contents.back() += toString(text); + contents.back() += text; } // Append a newline and start a new line void InputSources::appendNewline(std::string_view newline) { if (sealed) BUG("Appending to sealed InputSources"); - contents.back() += toString(newline); + contents.back() += newline; contents.push_back(""); // start a new line } @@ -135,9 +139,9 @@ void InputSources::appendText(const char *text) { } } -cstring InputSources::getLine(unsigned lineNumber) const { +std::string_view InputSources::getLine(unsigned lineNumber) const { if (lineNumber == 0) { - return ""_cs; + return ""; // BUG("Lines are numbered starting at 1"); // don't throw: this code may be called by exceptions // reporting on elements that have no source position @@ -185,13 +189,13 @@ cstring InputSources::getSourceFragment(const SourcePosition &position, bool use return getSourceFragment(info, useMarker); } -cstring carets(cstring source, unsigned start, unsigned end) { +static std::string carets(std::string_view source, unsigned start, unsigned end) { std::stringstream builder; if (start > source.size()) start = source.size(); - unsigned i; - for (i = 0; i < start; i++) { - char c = source.c_str()[i]; + unsigned i = 0; + for (; i < start; i++) { + char c = source[i]; if (c == ' ' || c == '\t') builder.put(c); else @@ -210,56 +214,54 @@ cstring InputSources::getSourceFragment(const SourceInfo &position, bool useMark if (position.getEnd().getLineNumber() > position.getStart().getLineNumber()) return getSourceFragment(position.getStart(), useMarker); - cstring result = getLine(position.getStart().getLineNumber()); - // Normally result has a newline, but if not - // then we have to add a newline - cstring toadd = ""_cs; - if (result.find('\n') == nullptr) toadd = cstring::newline; - cstring marker = - carets(result, position.getStart().getColumnNumber(), position.getEnd().getColumnNumber()); + std::string_view result = getLine(position.getStart().getLineNumber()); + unsigned int start = position.getStart().getColumnNumber(); + unsigned int end = position.getEnd().getColumnNumber(); + + // Normally result has a newline, but if not then we have to add a newline + bool addNewline = !absl::StrContains(result, "\n"); if (useMarker) { - return result + toadd + marker + cstring::newline; - } else { - return result + toadd + cstring::newline; + return absl::StrCat(result, addNewline ? "\n" : "", carets(result, start, end), "\n"); } + + return absl::StrCat(result, addNewline ? "\n" : "", "\n"); } cstring InputSources::getBriefSourceFragment(const SourceInfo &position) const { if (!position.isValid()) return ""_cs; - cstring result = getLine(position.getStart().getLineNumber()); + std::string_view result = getLine(position.getStart().getLineNumber()); unsigned int start = position.getStart().getColumnNumber(); - unsigned int end; - cstring toadd = ""_cs; + unsigned int end = position.getEnd().getColumnNumber(); + bool truncate = false; // If the position spans multiple lines, truncate to just the first line if (position.getEnd().getLineNumber() > position.getStart().getLineNumber()) { // go to the end of the first line end = result.size(); - if (result.find('\n') != nullptr) { + if (absl::StrContains(result, "\n")) { --end; } - toadd = " ..."_cs; - } else { - end = position.getEnd().getColumnNumber(); + truncate = true; } // Adding escape character in front of '"' character to properly store // quote marks as part of JSON properties, they must be escaped. - if (result.find('"') != nullptr) { - cstring out = result.replace("\""_cs, "\\\""_cs); + if (absl::StrContains(result, "\"")) { + auto out = absl::StrReplaceAll(result, {{"\"", "\\\""}}); return out.substr(0, out.size() - 1); } - return result.substr(start, end - start) + toadd; + return absl::StrCat(result.substr(start, end - start), truncate ? " ..." : ""); } cstring InputSources::toDebugString() const { std::stringstream builder; - for (auto line : contents) builder << line; + for (const auto &line : contents) builder << line; builder << "---------------" << std::endl; - for (auto lf : line_file_map) builder << lf.first << ": " << lf.second.toString() << std::endl; - return cstring(builder.str()); + for (const auto &lf : line_file_map) + builder << lf.first << ": " << lf.second.toString() << std::endl; + return builder.str(); } /////////////////////////////////////////////////// @@ -308,7 +310,7 @@ cstring SourceInfo::getLineNum() const { //////////////////////////////////////////////////////// cstring SourceFileLine::toString() const { - return Util::printf_format("%s(%d)", fileName.c_str(), sourceLine); + return absl::StrFormat("%s(%d)", fileName.string_view(), sourceLine); } } // namespace Util diff --git a/lib/source_file.h b/lib/source_file.h index 198db41c13..3abab5503c 100644 --- a/lib/source_file.h +++ b/lib/source_file.h @@ -22,6 +22,7 @@ limitations under the License. #define LIB_SOURCE_FILE_H_ #include +#include #include #include "cstring.h" @@ -267,7 +268,7 @@ class InputSources final { public: InputSources(); - cstring getLine(unsigned lineNumber) const; + std::string_view getLine(unsigned lineNumber) const; /// Original source line that produced the line with the specified number SourceFileLine getSourceLine(unsigned line) const; diff --git a/lib/stringify.cpp b/lib/stringify.cpp index cc113350c8..55a86eec8a 100644 --- a/lib/stringify.cpp +++ b/lib/stringify.cpp @@ -128,32 +128,4 @@ cstring toString(cstring value) { cstring toString(std::string_view value) { return cstring(value); } -cstring printf_format(const char *fmt_str, ...) { - if (fmt_str == nullptr) throw std::runtime_error("Null format string"); - va_list ap; - va_start(ap, fmt_str); - cstring formatted = vprintf_format(fmt_str, ap); - va_end(ap); - return formatted; -} - -// printf into a string -cstring vprintf_format(const char *fmt_str, va_list ap) { - static char buf[128]; - va_list ap_copy; - va_copy(ap_copy, ap); - if (fmt_str == nullptr) throw std::runtime_error("Null format string"); - - int size = vsnprintf(buf, sizeof(buf), fmt_str, ap); - if (size < 0) throw std::runtime_error("Error in vsnprintf"); - if (static_cast(size) >= sizeof(buf)) { - char *formatted = new char[size + 1]; - vsnprintf(formatted, size + 1, fmt_str, ap_copy); - va_end(ap_copy); - return cstring::own(formatted, size); - } - va_end(ap_copy); - return cstring(buf); -} - } // namespace Util diff --git a/lib/stringify.h b/lib/stringify.h index 51ffdcd03e..15cde6e449 100644 --- a/lib/stringify.h +++ b/lib/stringify.h @@ -95,10 +95,6 @@ cstring toString(std::string_view value); cstring toString(const big_int &value, unsigned width, bool sign, unsigned int base = 10); cstring toString(const void *value); -// printf into a string -cstring printf_format(const char *fmt_str, ...); -// vprintf into a string -cstring vprintf_format(const char *fmt_str, va_list ap); char DigitToChar(int digit); } // namespace Util diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 1c3c24235b..5969155959 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -51,7 +51,6 @@ set (GTEST_UNITTEST_SOURCES gtest/source_file_test.cpp gtest/strength_reduction.cpp gtest/transforms.cpp - gtest/stringify.cpp gtest/rtti_test.cpp gtest/nethash.cpp gtest/visitor.cpp diff --git a/test/gtest/format_test.cpp b/test/gtest/format_test.cpp index 6c32863c15..0be5dab2a6 100644 --- a/test/gtest/format_test.cpp +++ b/test/gtest/format_test.cpp @@ -53,9 +53,6 @@ TEST(Util, Format) { message = context.errorReporter().format_message("%1% %2%", x, 5); EXPECT_EQ("x 5\n", message); - - message = Util::printf_format("Number=%d, String=%s", 5, "short"); - EXPECT_EQ("Number=5, String=short", message); } } // namespace Util diff --git a/test/gtest/source_file_test.cpp b/test/gtest/source_file_test.cpp index bea02b7e52..2c34b03b25 100644 --- a/test/gtest/source_file_test.cpp +++ b/test/gtest/source_file_test.cpp @@ -67,10 +67,10 @@ TEST(UtilSourceFile, InputSources) { EXPECT_EQ(3u, sources.lineCount()); - cstring fl = sources.getLine(1); + auto fl = sources.getLine(1); EXPECT_EQ("First line\n", fl); - cstring sl = sources.getLine(2); + auto sl = sources.getLine(2); EXPECT_EQ("Second line\n", sl); SourceFileLine original = sources.getSourceLine(3); diff --git a/test/gtest/stringify.cpp b/test/gtest/stringify.cpp deleted file mode 100644 index 3abc316baf..0000000000 --- a/test/gtest/stringify.cpp +++ /dev/null @@ -1,69 +0,0 @@ -/* -Copyright 2018-present VMware, Inc. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ -#include "lib/stringify.h" - -#include - -#include - -#include "lib/cstring.h" - -namespace Test { -cstring appendFormat(const char *format, ...) { - va_list ap; - va_start(ap, format); - cstring str = Util::vprintf_format(format, ap); - va_end(ap); - return str; -} - -TEST(stringify, vprintf_simple) { - cstring str = appendFormat("%s", "AAA"); - EXPECT_EQ(str, "AAA"); -} - -TEST(stringify, vprintf_overflow) { - std::string c(129, 'A'); - cstring test_str = c; - cstring str = appendFormat("%s", test_str); - EXPECT_EQ(str.c_str(), test_str.c_str()); - EXPECT_EQ(str.size(), test_str.size()); -} - -TEST(stringify, vprintf_empty) { - cstring str = appendFormat("%s", ""); - EXPECT_EQ(str, ""); -} - -TEST(stringify, printf_simple) { - cstring str = Util::printf_format("%s", "AAA"); - EXPECT_EQ(str, "AAA"); -} - -TEST(stringify, printf_overflow) { - std::string c(129, 'A'); - cstring test_str = c; - cstring str = Util::printf_format("%s", test_str); - EXPECT_EQ(str.c_str(), test_str.c_str()); - EXPECT_EQ(str.size(), test_str.size()); -} - -TEST(stringify, printf_empty) { - cstring str = Util::printf_format("%s", ""); - EXPECT_EQ(str, ""); -} - -} // namespace Test