From 2a46fdb31d5526624052d62037a37329f84f21e6 Mon Sep 17 00:00:00 2001 From: "A.J. Beamon" Date: Fri, 4 May 2018 13:35:25 -0700 Subject: [PATCH] Change formatv to vsformat, which now returns the length of the string (or <0 for error) and takes the output string as an argument. --- flow/Trace.cpp | 55 +++++++++++++++++++++++++++----------------------- flow/Trace.h | 42 ++------------------------------------ flow/flow.cpp | 37 ++++++++++++++++----------------- flow/flow.h | 5 ++++- 4 files changed, 53 insertions(+), 86 deletions(-) diff --git a/flow/Trace.cpp b/flow/Trace.cpp index 469865cd25..4b05bcdb80 100644 --- a/flow/Trace.cpp +++ b/flow/Trace.cpp @@ -871,7 +871,7 @@ TraceEvent& TraceEvent::error(class Error const& error, bool includeCancelled) { return *this; } -TraceEvent& TraceEvent::detail( std::string key, std::string value ) { +TraceEvent& TraceEvent::detailImpl( std::string key, std::string value, bool writeEventMetricField) { if (enabled) { if( value.size() > 495 ) { value = value.substr(0, 495) + "..."; @@ -880,82 +880,87 @@ TraceEvent& TraceEvent::detail( std::string key, std::string value ) { fields.addField(key, value); length += key.size() + value.size(); // TODO: Overflow? buffer was first 300 bytes. TraceEvent(SevError, "TraceEventOverflow").detail("TraceFirstBytes", buffer); - tmpEventMetric->setField(key.c_str(), Standalone(StringRef(value))); + + if(writeEventMetricField) { + tmpEventMetric->setField(key.c_str(), Standalone(StringRef(value))); + } } return *this; } + +TraceEvent& TraceEvent::detail( std::string key, std::string value ) { + return detailImpl(key, value); +} TraceEvent& TraceEvent::detail( std::string key, double value ) { if(enabled) tmpEventMetric->setField(key.c_str(), value); - return detailf_no_metric( key, "%g", value ); + return detailfNoMetric( key, "%g", value ); } TraceEvent& TraceEvent::detail( std::string key, int value ) { if(enabled) tmpEventMetric->setField(key.c_str(), (int64_t)value); - return detailf_no_metric( key, "%d", value ); + return detailfNoMetric( key, "%d", value ); } TraceEvent& TraceEvent::detail( std::string key, unsigned value ) { if(enabled) tmpEventMetric->setField(key.c_str(), (int64_t)value); - return detailf_no_metric( key, "%u", value ); + return detailfNoMetric( key, "%u", value ); } TraceEvent& TraceEvent::detail( std::string key, long int value ) { if(enabled) tmpEventMetric->setField(key.c_str(), (int64_t)value); - return detailf_no_metric( key, "%ld", value ); + return detailfNoMetric( key, "%ld", value ); } TraceEvent& TraceEvent::detail( std::string key, long unsigned int value ) { if(enabled) tmpEventMetric->setField(key.c_str(), (int64_t)value); - return detailf_no_metric( key, "%lu", value ); + return detailfNoMetric( key, "%lu", value ); } TraceEvent& TraceEvent::detail( std::string key, long long int value ) { if(enabled) tmpEventMetric->setField(key.c_str(), (int64_t)value); - return detailf_no_metric( key, "%lld", value ); + return detailfNoMetric( key, "%lld", value ); } TraceEvent& TraceEvent::detail( std::string key, long long unsigned int value ) { if(enabled) tmpEventMetric->setField(key.c_str(), (int64_t)value); - return detailf_no_metric( key, "%llu", value ); + return detailfNoMetric( key, "%llu", value ); } TraceEvent& TraceEvent::detail( std::string key, NetworkAddress const& value ) { - return detail( key, value.toString() ); + return detailImpl( key, value.toString() ); } TraceEvent& TraceEvent::detail( std::string key, UID const& value ) { return detailf( key, "%016llx", value.first() ); // SOMEDAY: Log entire value? We also do this explicitly in some "lists" in various individual TraceEvent calls } TraceEvent& TraceEvent::detailext( std::string key, StringRef const& value ) { - return detail(key, value.printable()); + return detailImpl(key, value.printable()); } TraceEvent& TraceEvent::detailext( std::string key, Optional> const& value ) { - return detail(key, (value.present()) ? value.get().printable() : "[not set]"); + return detailImpl(key, (value.present()) ? value.get().printable() : "[not set]"); } TraceEvent& TraceEvent::detailf( std::string key, const char* valueFormat, ... ) { if (enabled) { va_list args; va_start(args, valueFormat); - detailfv( key, valueFormat, args, true); // Write this detail to eventMetric + std::string value; + int result = vsformat(value, valueFormat, args); va_end(args); + + ASSERT(result >= 0); + detailImpl(key, value); } return *this; } -TraceEvent& TraceEvent::detailf_no_metric( std::string key, const char* valueFormat, ... ) { +TraceEvent& TraceEvent::detailfNoMetric( std::string key, const char* valueFormat, ... ) { if (enabled) { va_list args; va_start(args, valueFormat); - detailfv( key, valueFormat, args, false); // Do NOT write this detail to the event metric, caller of detailf_no_metric should do that itself with the appropriate value type + std::string value; + int result = vsformat(value, valueFormat, args); va_end(args); - } - return *this; -} -TraceEvent& TraceEvent::detailfv( std::string key, const char* valueFormat, va_list args, bool writeEventMetricField ) { - if (enabled) { - std::string value = formatv(valueFormat, args); // TODO: exceptions? - fields.addField(key, value); - if(writeEventMetricField) { - tmpEventMetric->setField(key.c_str(), Standalone(StringRef(value))); - } + + ASSERT(result >= 0); + detailImpl(key, value, false); // Do NOT write this detail to the event metric, caller of detailfNoMetric should do that itself with the appropriate value type } return *this; } diff --git a/flow/Trace.h b/flow/Trace.h index 00eb86e70d..05e9f0a7f4 100644 --- a/flow/Trace.h +++ b/flow/Trace.h @@ -129,7 +129,6 @@ class StringRef; template class Standalone; template class Optional; -#if 1 struct TraceEvent { TraceEvent( const char* type, UID id = UID() ); // Assumes SevInfo severity TraceEvent( Severity, const char* type, UID id = UID() ); @@ -161,9 +160,9 @@ struct TraceEvent { private: // Private version of detailf that does NOT write to the eventMetric. This is to be used by other detail methods // which can write field metrics of a more appropriate type than string but use detailf() to add to the TraceEvent. - TraceEvent& detailf_no_metric( std::string key, const char* valueFormat, ... ); + TraceEvent& detailfNoMetric( std::string key, const char* valueFormat, ... ); + TraceEvent& detailImpl( std::string key, std::string value, bool writeEventMetricField=true ); public: - TraceEvent& detailfv( std::string key, const char* valueFormat, va_list args, bool writeEventMetricField ); TraceEvent& detail( std::string key, UID const& value ); TraceEvent& backtrace(std::string prefix = ""); TraceEvent& trackLatest( const char* trackingKey ); @@ -193,44 +192,7 @@ private: bool init( Severity, const char* type ); bool init( Severity, struct TraceInterval& ); - - void write( int length, const void* data ); - void writef( const char* format, ... ); - void writeEscaped( const char* data ); - void writeEscapedfv( const char* format, va_list args ); }; -#else -struct TraceEvent { - TraceEvent(const char* type, UID id = UID()) {} - TraceEvent(Severity, const char* type, UID id = UID()) {} - TraceEvent(struct TraceInterval&, UID id = UID()) {} - TraceEvent(const char* type, StringRef& const id); {} // Assumes SevInfo severity - TraceEvent(Severity, const char* type, StringRef& const id); {} - - static bool isEnabled(const char* type) { return false; } - - TraceEvent& error(class Error const& e, bool includeCancelled = false) { return *this; } - - TraceEvent& detail(std::string key, std::string value) { return *this; } - TraceEvent& detail(std::string key, double value) { return *this; } - TraceEvent& detail(std::string key, long int value) { return *this; } - TraceEvent& detail(std::string key, long unsigned int value) { return *this; } - TraceEvent& detail(std::string key, long long int value) { return *this; } - TraceEvent& detail(std::string key, long long unsigned int value) { return *this; } - TraceEvent& detail(std::string key, int value) { return *this; } - TraceEvent& detail(std::string key, unsigned value) { return *this; } - TraceEvent& detail(std::string key, struct NetworkAddress const& value) { return *this; } - TraceEvent& detailf(std::string key, const char* valueFormat, ...) { return *this; } - TraceEvent& detailfv(std::string key, const char* valueFormat, va_list args) { return *this; } - TraceEvent& detail(std::string key, UID const& value) { return *this; } - TraceEvent& detailext(std::string key, StringRef const& value) { return *this; } - TraceEvent& detailext(std::string key, Optional> const& value); { return *this; } - TraceEvent& backtrace(std::string prefix = "") { return *this; } - TraceEvent& trackLatest(const char* trackingKey) { return *this; } - - TraceEvent& GetLastError() { return *this; } -}; -#endif struct TraceLogFormatter { virtual const char* getExtension() = 0; diff --git a/flow/flow.cpp b/flow/flow.cpp index 1f90390d0a..264d912361 100644 --- a/flow/flow.cpp +++ b/flow/flow.cpp @@ -93,8 +93,7 @@ Optional parse_with_suffix(std::string toparse, std::string default_un return ret; } -// TODO: Is there a better way to implement format/formatv? -std::string formatv( const char* form, va_list args) { +int vsformat( std::string &outputString, const char* form, va_list args) { char buf[200]; va_list args2; @@ -103,7 +102,8 @@ std::string formatv( const char* form, va_list args) { va_end(args2); if(size >= 0 && size < sizeof(buf)) { - return std::string(buf, size); + outputString = std::string(buf, size); + return size; } #ifdef _WIN32 @@ -113,35 +113,32 @@ std::string formatv( const char* form, va_list args) { va_end(args2); #endif - if (size < 0) throw internal_error(); + if (size < 0) { + return -1; + } TEST(true); //large format result - std::string s; - s.resize(size + 1); - size = vsnprintf(&s[0], s.size(), form, args); - if (size < 0 || size >= s.size()) throw internal_error(); + outputString.resize(size + 1); + size = vsnprintf(&outputString[0], outputString.size(), form, args); + if (size < 0 || size >= outputString.size()) { + return -1; + } - s.resize(size); - return s; + outputString.resize(size); + return size; } std::string format( const char* form, ... ) { va_list args; va_start(args, form); - std::string result; - try { - result = formatv(form, args); - } - catch(Error &e) { - va_end(args); - throw; - } - + std::string str; + int result = vsformat(str, form, args); va_end(args); - return result; + ASSERT(result >= 0); + return str; } Standalone strinc(StringRef const& str) { diff --git a/flow/flow.h b/flow/flow.h index f4f2bc7888..609f72b41d 100644 --- a/flow/flow.h +++ b/flow/flow.h @@ -68,7 +68,10 @@ bool validationIsEnabled(); extern Optional parse_with_suffix(std::string toparse, std::string default_unit = ""); extern std::string format(const char* form, ...); -extern std::string formatv(const char* form, va_list args); + +// On success, returns the number of characters written. On failure, returns a negative number. +extern int vsformat(std::string &outputString, const char* form, va_list args); + extern Standalone strinc(StringRef const& str); extern StringRef strinc(StringRef const& str, Arena& arena);