Skip to content

Commit

Permalink
Cleanup package_string() and its users
Browse files Browse the repository at this point in the history
package_string() isn't readable in its current form and a loop is
unnecessary, so let's replace that with the direct calculations.  The
new and old functions are identical in results except an edge case
where the old function incorrectly believes it needs to round up to
'10' for the size prefix, when '9' would be ok, specifically:
10\naaaaa\n\f vs 9\naaaaa\n\f.  This is true for all powers of 10.

Clean up the calling side in logcat as well.

Test: printing log statistics and prune list works
Change-Id: Ib62ab2badab59040215b130ec9e3efbc7c95af3f
  • Loading branch information
Tom Cherry committed Dec 6, 2019
1 parent ed860ff commit e17b4f6
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 42 deletions.
53 changes: 24 additions & 29 deletions logcat/logcat.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1082,50 +1082,45 @@ int Logcat::Run(int argc, char** argv) {
}

if (printStatistics || getPruneList) {
size_t len = 8192;
char* buf;
std::string buf(8192, '\0');
size_t ret_length = 0;
int retry = 32;

for (int retry = 32; (retry >= 0) && ((buf = new char[len]));
delete[] buf, buf = nullptr, --retry) {
for (; retry >= 0; --retry) {
if (getPruneList) {
android_logger_get_prune_list(logger_list.get(), buf, len);
android_logger_get_prune_list(logger_list.get(), buf.data(), buf.size());
} else {
android_logger_get_statistics(logger_list.get(), buf, len);
android_logger_get_statistics(logger_list.get(), buf.data(), buf.size());
}
buf[len - 1] = '\0';
if (atol(buf) < 3) {
delete[] buf;
buf = nullptr;
break;

ret_length = atol(buf.c_str());
if (ret_length < 3) {
error(EXIT_FAILURE, 0, "Failed to read data.");
}
size_t ret = atol(buf) + 1;
if (ret <= len) {
len = ret;

if (ret_length < buf.size()) {
break;
}
len = ret;

buf.resize(ret_length + 1);
}

if (!buf) {
if (retry < 0) {
error(EXIT_FAILURE, 0, "Failed to read data.");
}

// remove trailing FF
char* cp = buf + len - 1;
*cp = '\0';
bool truncated = *--cp != '\f';
if (!truncated) *cp = '\0';

// squash out the byte count
cp = buf;
if (!truncated) {
while (isdigit(*cp)) ++cp;
if (*cp == '\n') ++cp;
buf.resize(ret_length);
if (buf.back() == '\f') {
buf.pop_back();
}

len = strlen(cp);
// Remove the byte count prefix
const char* cp = buf.c_str();
while (isdigit(*cp)) ++cp;
if (*cp == '\n') ++cp;

size_t len = strlen(cp);
TEMP_FAILURE_RETRY(write(output_fd_.get(), cp, len));
delete[] buf;
return EXIT_SUCCESS;
}

Expand Down
37 changes: 24 additions & 13 deletions logd/CommandListener.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <dirent.h>
#include <errno.h>
#include <fcntl.h>
#include <math.h>
#include <netinet/in.h>
#include <stdlib.h>
#include <string.h>
Expand Down Expand Up @@ -186,14 +187,26 @@ CommandListener::GetStatisticsCmd::GetStatisticsCmd(LogBuffer* buf)
: LogCommand("getStatistics"), mBuf(*buf) {
}

static std::string package_string(const std::string& str) {
// Calculate total buffer size prefix, count is the string length w/o nul
char fmt[32];
for (size_t l = str.length(), y = 0, x = 6; y != x;
y = x, x = strlen(fmt) - 2) {
snprintf(fmt, sizeof(fmt), "%zu\n%%s\n\f", l + x);
// This returns a string with a length prefix with the format <length>\n<data>\n\f. The length
// prefix includes the length of the prefix itself.
static std::string PackageString(const std::string& str) {
size_t overhead_length = 3; // \n \n \f.

// Number of digits needed to represent length(str + overhead_length).
size_t str_size_digits = 1 + static_cast<size_t>(log10(str.size() + overhead_length));
// Number of digits needed to represent the total size.
size_t total_size_digits =
1 + static_cast<size_t>(log10(str.size() + overhead_length + str_size_digits));

// If adding the size prefix causes a new digit to be required to represent the new total
// size, add it to the 'overhead_length'. This can only happen once, since each new digit
// allows for 10x the previous size to be recorded.
if (total_size_digits != str_size_digits) {
overhead_length++;
}
return android::base::StringPrintf(fmt, str.c_str());

size_t total_size = str.size() + overhead_length + str_size_digits;
return android::base::StringPrintf("%zu\n%s\n\f", total_size, str.c_str());
}

int CommandListener::GetStatisticsCmd::runCommand(SocketClient* cli, int argc,
Expand Down Expand Up @@ -228,8 +241,7 @@ int CommandListener::GetStatisticsCmd::runCommand(SocketClient* cli, int argc,
}
}

cli->sendMsg(
package_string(mBuf.formatStatistics(uid, pid, logMask)).c_str());
cli->sendMsg(PackageString(mBuf.formatStatistics(uid, pid, logMask)).c_str());
return 0;
}

Expand All @@ -240,7 +252,7 @@ CommandListener::GetPruneListCmd::GetPruneListCmd(LogBuffer* buf)
int CommandListener::GetPruneListCmd::runCommand(SocketClient* cli,
int /*argc*/, char** /*argv*/) {
setname();
cli->sendMsg(package_string(mBuf.formatPrune()).c_str());
cli->sendMsg(PackageString(mBuf.formatPrune()).c_str());
return 0;
}

Expand Down Expand Up @@ -316,12 +328,11 @@ int CommandListener::GetEventTagCmd::runCommand(SocketClient* cli, int argc,
cli->sendMsg("can not mix id= with either format= or name=");
return 0;
}
cli->sendMsg(package_string(mBuf.formatEntry(atoi(id), uid)).c_str());
cli->sendMsg(PackageString(mBuf.formatEntry(atoi(id), uid)).c_str());
return 0;
}

cli->sendMsg(
package_string(mBuf.formatGetEventTag(uid, name, format)).c_str());
cli->sendMsg(PackageString(mBuf.formatGetEventTag(uid, name, format)).c_str());

return 0;
}
Expand Down

0 comments on commit e17b4f6

Please sign in to comment.