logcat: more error printing clean-up

* The --help text is way too long to print after each error, so simply
  print the errors and exit.
* Report errno in a few cases where it was previously not reported
* Fix more punctuation
* Fix '?' and ':' getopt_long() return cases for long options.

Test: errors look better
Change-Id: I57058a2250e9f3c3431f104e43f0eb5ec60d8c8a
This commit is contained in:
Tom Cherry 2019-12-06 10:25:37 -08:00
parent 1dd1056f5d
commit 72a4e08864
1 changed files with 57 additions and 80 deletions

View File

@ -17,6 +17,7 @@
#include <ctype.h> #include <ctype.h>
#include <dirent.h> #include <dirent.h>
#include <errno.h> #include <errno.h>
#include <error.h>
#include <fcntl.h> #include <fcntl.h>
#include <getopt.h> #include <getopt.h>
#include <math.h> #include <math.h>
@ -108,10 +109,6 @@ class Logcat {
enum helpType { HELP_FALSE, HELP_TRUE, HELP_FORMAT }; enum helpType { HELP_FALSE, HELP_TRUE, HELP_FORMAT };
// if show_help is set, newline required in fmt statement to transition to usage
static void LogcatPanic(enum helpType showHelp, const char* fmt, ...) __printflike(2, 3)
__attribute__((__noreturn__));
#ifndef F2FS_IOC_SET_PIN_FILE #ifndef F2FS_IOC_SET_PIN_FILE
#define F2FS_IOCTL_MAGIC 0xf5 #define F2FS_IOCTL_MAGIC 0xf5
#define F2FS_IOC_SET_PIN_FILE _IOW(F2FS_IOCTL_MAGIC, 13, __u32) #define F2FS_IOC_SET_PIN_FILE _IOW(F2FS_IOCTL_MAGIC, 13, __u32)
@ -170,7 +167,7 @@ void Logcat::RotateLogs() {
output_fd_.reset(openLogFile(output_file_name_, log_rotate_size_kb_)); output_fd_.reset(openLogFile(output_file_name_, log_rotate_size_kb_));
if (!output_fd_.ok()) { if (!output_fd_.ok()) {
LogcatPanic(HELP_FALSE, "couldn't open output file"); error(EXIT_FAILURE, errno, "Couldn't open output file");
} }
out_byte_count_ = 0; out_byte_count_ = 0;
@ -209,7 +206,7 @@ void Logcat::ProcessBuffer(struct log_msg* buf) {
bytesWritten = android_log_printLogLine(logformat_.get(), output_fd_.get(), &entry); bytesWritten = android_log_printLogLine(logformat_.get(), output_fd_.get(), &entry);
if (bytesWritten < 0) { if (bytesWritten < 0) {
LogcatPanic(HELP_FALSE, "output error"); error(EXIT_FAILURE, 0, "Output error.");
} }
} }
} }
@ -229,7 +226,7 @@ void Logcat::PrintDividers(log_id_t log_id, bool print_dividers) {
if (dprintf(output_fd_.get(), "--------- %s %s\n", if (dprintf(output_fd_.get(), "--------- %s %s\n",
printed_start_[log_id] ? "switch to" : "beginning of", printed_start_[log_id] ? "switch to" : "beginning of",
android_log_id_to_name(log_id)) < 0) { android_log_id_to_name(log_id)) < 0) {
LogcatPanic(HELP_FALSE, "output error"); error(EXIT_FAILURE, errno, "Output error");
} }
} }
last_printed_id_ = log_id; last_printed_id_ = log_id;
@ -259,18 +256,16 @@ void Logcat::SetupOutputAndSchedulingPolicy(bool blocking) {
output_fd_.reset(openLogFile(output_file_name_, log_rotate_size_kb_)); output_fd_.reset(openLogFile(output_file_name_, log_rotate_size_kb_));
if (!output_fd_.ok()) { if (!output_fd_.ok()) {
LogcatPanic(HELP_FALSE, "couldn't open output file"); error(EXIT_FAILURE, errno, "Couldn't open output file");
} }
struct stat statbuf; struct stat statbuf;
if (fstat(output_fd_.get(), &statbuf) == -1) { if (fstat(output_fd_.get(), &statbuf) == -1) {
output_fd_.reset(); error(EXIT_FAILURE, errno, "Couldn't get output file stat");
LogcatPanic(HELP_FALSE, "couldn't get output file stat\n");
} }
if ((size_t)statbuf.st_size > SIZE_MAX || statbuf.st_size < 0) { if ((size_t)statbuf.st_size > SIZE_MAX || statbuf.st_size < 0) {
output_fd_.reset(); error(EXIT_FAILURE, 0, "Invalid output file stat.");
LogcatPanic(HELP_FALSE, "invalid output file stat\n");
} }
out_byte_count_ = statbuf.st_size; out_byte_count_ = statbuf.st_size;
@ -427,27 +422,6 @@ static std::pair<unsigned long, const char*> format_of_size(unsigned long value)
return std::make_pair(value, multipliers[i]); return std::make_pair(value, multipliers[i]);
} }
static void LogcatPanic(enum helpType showHelp, const char* fmt, ...) {
va_list args;
va_start(args, fmt);
vfprintf(stderr, fmt, args);
va_end(args);
switch (showHelp) {
case HELP_TRUE:
show_help();
break;
case HELP_FORMAT:
show_format_help();
break;
case HELP_FALSE:
default:
break;
}
exit(EXIT_FAILURE);
}
static char* parseTime(log_time& t, const char* cp) { static char* parseTime(log_time& t, const char* cp) {
char* ep = t.strptime(cp, "%m-%d %H:%M:%S.%q"); char* ep = t.strptime(cp, "%m-%d %H:%M:%S.%q");
if (ep) return ep; if (ep) return ep;
@ -612,13 +586,12 @@ int Logcat::Run(int argc, char** argv) {
// only long options // only long options
if (long_options[option_index].name == pid_str) { if (long_options[option_index].name == pid_str) {
if (pid != 0) { if (pid != 0) {
LogcatPanic(HELP_TRUE, "Only supports one PID argument.\n"); error(EXIT_FAILURE, 0, "Only one --pid argument can be provided.");
} }
// ToDo: determine runtime PID_MAX?
if (!ParseUint(optarg, &pid) || pid < 1) { if (!ParseUint(optarg, &pid) || pid < 1) {
LogcatPanic(HELP_TRUE, "%s %s out of range\n", error(EXIT_FAILURE, 0, "%s %s out of range.",
long_options[option_index].name, optarg); long_options[option_index].name, optarg);
} }
break; break;
} }
@ -628,8 +601,8 @@ int Logcat::Run(int argc, char** argv) {
// ToDo: implement API that supports setting a wrap timeout // ToDo: implement API that supports setting a wrap timeout
size_t dummy = ANDROID_LOG_WRAP_DEFAULT_TIMEOUT; size_t dummy = ANDROID_LOG_WRAP_DEFAULT_TIMEOUT;
if (optarg && (!ParseUint(optarg, &dummy) || dummy < 1)) { if (optarg && (!ParseUint(optarg, &dummy) || dummy < 1)) {
LogcatPanic(HELP_TRUE, "%s %s out of range\n", error(EXIT_FAILURE, 0, "%s %s out of range.",
long_options[option_index].name, optarg); long_options[option_index].name, optarg);
} }
if (dummy != ANDROID_LOG_WRAP_DEFAULT_TIMEOUT) { if (dummy != ANDROID_LOG_WRAP_DEFAULT_TIMEOUT) {
fprintf(stderr, "WARNING: %s %u seconds, ignoring %zu\n", fprintf(stderr, "WARNING: %s %u seconds, ignoring %zu\n",
@ -678,13 +651,13 @@ int Logcat::Run(int argc, char** argv) {
if (strspn(optarg, "0123456789") != strlen(optarg)) { if (strspn(optarg, "0123456789") != strlen(optarg)) {
char* cp = parseTime(tail_time, optarg); char* cp = parseTime(tail_time, optarg);
if (!cp) { if (!cp) {
LogcatPanic(HELP_FALSE, "-%c \"%s\" not in time format\n", c, optarg); error(EXIT_FAILURE, 0, "-%c '%s' not in time format.", c, optarg);
} }
if (*cp) { if (*cp) {
char ch = *cp; char ch = *cp;
*cp = '\0'; *cp = '\0';
fprintf(stderr, "WARNING: -%c \"%s\"\"%c%s\" time truncated\n", c, optarg, fprintf(stderr, "WARNING: -%c '%s' '%c%s' time truncated\n", c, optarg, ch,
ch, cp + 1); cp + 1);
*cp = ch; *cp = ch;
} }
} else { } else {
@ -705,8 +678,8 @@ int Logcat::Run(int argc, char** argv) {
case 'm': { case 'm': {
if (!ParseUint(optarg, &max_count_) || max_count_ < 1) { if (!ParseUint(optarg, &max_count_) || max_count_ < 1) {
LogcatPanic(HELP_FALSE, "-%c \"%s\" isn't an integer greater than zero\n", c, error(EXIT_FAILURE, 0, "-%c '%s' isn't an integer greater than zero.", c,
optarg); optarg);
} }
} break; } break;
@ -719,7 +692,7 @@ int Logcat::Run(int argc, char** argv) {
case 'G': { case 'G': {
if (!ParseByteCount(optarg, &setLogSize) || setLogSize < 1) { if (!ParseByteCount(optarg, &setLogSize) || setLogSize < 1) {
LogcatPanic(HELP_FALSE, "ERROR: -G <num><multiplier>\n"); error(EXIT_FAILURE, 0, "-G must be specified as <num><multiplier>.");
} }
} break; } break;
@ -743,7 +716,8 @@ int Logcat::Run(int argc, char** argv) {
} else { } else {
log_id_t log_id = android_name_to_log_id(buffer.c_str()); log_id_t log_id = android_name_to_log_id(buffer.c_str());
if (log_id >= LOG_ID_MAX) { if (log_id >= LOG_ID_MAX) {
LogcatPanic(HELP_TRUE, "unknown buffer %s\n", buffer.c_str()); error(EXIT_FAILURE, 0, "Unknown buffer '%s' listed for -b.",
buffer.c_str());
} }
if (log_id == LOG_ID_SECURITY) { if (log_id == LOG_ID_SECURITY) {
security_buffer_selected = true; security_buffer_selected = true;
@ -767,13 +741,13 @@ int Logcat::Run(int argc, char** argv) {
case 'r': case 'r':
if (!ParseUint(optarg, &log_rotate_size_kb_) || log_rotate_size_kb_ < 1) { if (!ParseUint(optarg, &log_rotate_size_kb_) || log_rotate_size_kb_ < 1) {
LogcatPanic(HELP_TRUE, "Invalid parameter \"%s\" to -r\n", optarg); error(EXIT_FAILURE, 0, "Invalid parameter '%s' to -r.", optarg);
} }
break; break;
case 'n': case 'n':
if (!ParseUint(optarg, &max_rotated_logs_) || max_rotated_logs_ < 1) { if (!ParseUint(optarg, &max_rotated_logs_) || max_rotated_logs_ < 1) {
LogcatPanic(HELP_TRUE, "Invalid parameter \"%s\" to -n\n", optarg); error(EXIT_FAILURE, 0, "Invalid parameter '%s' to -n.", optarg);
} }
break; break;
@ -785,7 +759,7 @@ int Logcat::Run(int argc, char** argv) {
for (const auto& arg : Split(optarg, delimiters)) { for (const auto& arg : Split(optarg, delimiters)) {
int err = SetLogFormat(arg.c_str()); int err = SetLogFormat(arg.c_str());
if (err < 0) { if (err < 0) {
LogcatPanic(HELP_FORMAT, "Invalid parameter \"%s\" to -v\n", arg.c_str()); error(EXIT_FAILURE, 0, "Invalid parameter '%s' to -v.", arg.c_str());
} }
if (err) hasSetLogFormat = true; if (err) hasSetLogFormat = true;
} }
@ -882,20 +856,25 @@ int Logcat::Run(int argc, char** argv) {
break; break;
case ':': case ':':
LogcatPanic(HELP_TRUE, "Option -%c needs an argument\n", optopt); error(EXIT_FAILURE, 0, "Option '%s' needs an argument.", argv[optind - 1]);
break;
case 'h': case 'h':
show_help(); show_help();
show_format_help(); show_format_help();
return EXIT_SUCCESS; return EXIT_SUCCESS;
case '?':
error(EXIT_FAILURE, 0, "Unknown option '%s'.", argv[optind - 1]);
break;
default: default:
LogcatPanic(HELP_TRUE, "Unrecognized Option %c\n", optopt); error(EXIT_FAILURE, 0, "Unknown getopt_long() result '%c'.", c);
} }
} }
if (max_count_ && got_t) { if (max_count_ && got_t) {
LogcatPanic(HELP_TRUE, "Cannot use -m (--max-count) and -t together\n"); error(EXIT_FAILURE, 0, "Cannot use -m (--max-count) and -t together.");
} }
if (print_it_anyways_ && (!regex_ || !max_count_)) { if (print_it_anyways_ && (!regex_ || !max_count_)) {
// One day it would be nice if --print -v color and --regex <expr> // One day it would be nice if --print -v color and --regex <expr>
@ -915,12 +894,12 @@ int Logcat::Run(int argc, char** argv) {
} }
if (log_rotate_size_kb_ != 0 && !output_file_name_) { if (log_rotate_size_kb_ != 0 && !output_file_name_) {
LogcatPanic(HELP_TRUE, "-r requires -f as well\n"); error(EXIT_FAILURE, 0, "-r requires -f as well.");
} }
if (setId != 0) { if (setId != 0) {
if (!output_file_name_) { if (!output_file_name_) {
LogcatPanic(HELP_TRUE, "--id='%s' requires -f as well\n", setId); error(EXIT_FAILURE, 0, "--id='%s' requires -f as well.", setId);
} }
std::string file_name = StringPrintf("%s.id", output_file_name_); std::string file_name = StringPrintf("%s.id", output_file_name_);
@ -952,7 +931,7 @@ int Logcat::Run(int argc, char** argv) {
if (forceFilters.size()) { if (forceFilters.size()) {
int err = android_log_addFilterString(logformat_.get(), forceFilters.c_str()); int err = android_log_addFilterString(logformat_.get(), forceFilters.c_str());
if (err < 0) { if (err < 0) {
LogcatPanic(HELP_FALSE, "Invalid filter expression in logcat args\n"); error(EXIT_FAILURE, 0, "Invalid filter expression in logcat args.");
} }
} else if (argc == optind) { } else if (argc == optind) {
// Add from environment variable // Add from environment variable
@ -962,7 +941,7 @@ int Logcat::Run(int argc, char** argv) {
int err = android_log_addFilterString(logformat_.get(), env_tags_orig); int err = android_log_addFilterString(logformat_.get(), env_tags_orig);
if (err < 0) { if (err < 0) {
LogcatPanic(HELP_TRUE, "Invalid filter expression in ANDROID_LOG_TAGS\n"); error(EXIT_FAILURE, 0, "Invalid filter expression in ANDROID_LOG_TAGS.");
} }
} }
} else { } else {
@ -970,17 +949,17 @@ int Logcat::Run(int argc, char** argv) {
for (int i = optind ; i < argc ; i++) { for (int i = optind ; i < argc ; i++) {
int err = android_log_addFilterString(logformat_.get(), argv[i]); int err = android_log_addFilterString(logformat_.get(), argv[i]);
if (err < 0) { if (err < 0) {
LogcatPanic(HELP_TRUE, "Invalid filter expression '%s'\n", argv[i]); error(EXIT_FAILURE, 0, "Invalid filter expression '%s'.", argv[i]);
} }
} }
} }
if (mode & ANDROID_LOG_PSTORE) { if (mode & ANDROID_LOG_PSTORE) {
if (output_file_name_) { if (output_file_name_) {
LogcatPanic(HELP_FALSE, "-c is ambiguous with both -f and -L specified.\n"); error(EXIT_FAILURE, 0, "-c is ambiguous with both -f and -L specified.");
} }
if (setLogSize || getLogSize || printStatistics || getPruneList || setPruneList) { if (setLogSize || getLogSize || printStatistics || getPruneList || setPruneList) {
LogcatPanic(HELP_TRUE, "-L is incompatible with -g/-G, -S, and -p/-P\n"); error(EXIT_FAILURE, 0, "-L is incompatible with -g/-G, -S, and -p/-P.");
} }
if (clearLog) { if (clearLog) {
unlink("/sys/fs/pstore/pmsg-ramoops-0"); unlink("/sys/fs/pstore/pmsg-ramoops-0");
@ -990,7 +969,7 @@ int Logcat::Run(int argc, char** argv) {
if (output_file_name_) { if (output_file_name_) {
if (setLogSize || getLogSize || printStatistics || getPruneList || setPruneList) { if (setLogSize || getLogSize || printStatistics || getPruneList || setPruneList) {
LogcatPanic(HELP_TRUE, "-f is incompatible with -g/-G, -S, and -p/-P\n"); error(EXIT_FAILURE, 0, "-f is incompatible with -g/-G, -S, and -p/-P.");
} }
if (clearLog || setId) { if (clearLog || setId) {
@ -1078,21 +1057,20 @@ int Logcat::Run(int argc, char** argv) {
// report any errors in the above loop and exit // report any errors in the above loop and exit
if (!open_device_failures.empty()) { if (!open_device_failures.empty()) {
LogcatPanic(HELP_FALSE, "Unable to open log device%s '%s'\n", error(EXIT_FAILURE, 0, "Unable to open log device%s '%s'.",
open_device_failures.size() > 1 ? "s" : "", open_device_failures.size() > 1 ? "s" : "", Join(open_device_failures, ",").c_str());
Join(open_device_failures, ",").c_str());
} }
if (!clear_failures.empty()) { if (!clear_failures.empty()) {
LogcatPanic(HELP_FALSE, "failed to clear the '%s' log%s\n", error(EXIT_FAILURE, 0, "failed to clear the '%s' log%s.", Join(clear_failures, ",").c_str(),
Join(clear_failures, ",").c_str(), clear_failures.size() > 1 ? "s" : ""); clear_failures.size() > 1 ? "s" : "");
} }
if (!set_size_failures.empty()) { if (!set_size_failures.empty()) {
LogcatPanic(HELP_FALSE, "failed to set the '%s' log size%s\n", error(EXIT_FAILURE, 0, "failed to set the '%s' log size%s.",
Join(set_size_failures, ",").c_str(), set_size_failures.size() > 1 ? "s" : ""); Join(set_size_failures, ",").c_str(), set_size_failures.size() > 1 ? "s" : "");
} }
if (!get_size_failures.empty()) { if (!get_size_failures.empty()) {
LogcatPanic(HELP_FALSE, "failed to get the readable '%s' log size%s\n", error(EXIT_FAILURE, 0, "failed to get the readable '%s' log size%s.",
Join(get_size_failures, ",").c_str(), get_size_failures.size() > 1 ? "s" : ""); Join(get_size_failures, ",").c_str(), get_size_failures.size() > 1 ? "s" : "");
} }
if (setPruneList) { if (setPruneList) {
@ -1103,11 +1081,11 @@ int Logcat::Run(int argc, char** argv) {
if (asprintf(&buf, "%-*s", (int)(bLen - 1), setPruneList) > 0) { if (asprintf(&buf, "%-*s", (int)(bLen - 1), setPruneList) > 0) {
buf[len] = '\0'; buf[len] = '\0';
if (android_logger_set_prune_list(logger_list.get(), buf, bLen)) { if (android_logger_set_prune_list(logger_list.get(), buf, bLen)) {
LogcatPanic(HELP_FALSE, "failed to set the prune list"); error(EXIT_FAILURE, 0, "Failed to set the prune list.");
} }
free(buf); free(buf);
} else { } else {
LogcatPanic(HELP_FALSE, "failed to set the prune list (alloc)"); error(EXIT_FAILURE, 0, "Failed to set the prune list (alloc).");
} }
return EXIT_SUCCESS; return EXIT_SUCCESS;
} }
@ -1138,7 +1116,7 @@ int Logcat::Run(int argc, char** argv) {
} }
if (!buf) { if (!buf) {
LogcatPanic(HELP_FALSE, "failed to read data"); error(EXIT_FAILURE, 0, "Failed to read data.");
} }
// remove trailing FF // remove trailing FF
@ -1168,30 +1146,29 @@ int Logcat::Run(int argc, char** argv) {
struct log_msg log_msg; struct log_msg log_msg;
int ret = android_logger_list_read(logger_list.get(), &log_msg); int ret = android_logger_list_read(logger_list.get(), &log_msg);
if (!ret) { if (!ret) {
LogcatPanic(HELP_FALSE, R"init(read: unexpected EOF! error(EXIT_FAILURE, 0, R"init(Unexpected EOF!
This means that either logd crashed, or more likely, this instance of logcat was unable to read log This means that either logd crashed, or more likely, this instance of logcat was unable to read log
messages as quickly as they were being produced. messages as quickly as they were being produced.
If you have enabled significant logging, look into using the -G option to increase log buffer sizes. If you have enabled significant logging, look into using the -G option to increase log buffer sizes.)init");
)init");
} }
if (ret < 0) { if (ret < 0) {
if (ret == -EAGAIN) break; if (ret == -EAGAIN) break;
if (ret == -EIO) { if (ret == -EIO) {
LogcatPanic(HELP_FALSE, "read: unexpected EOF!\n"); error(EXIT_FAILURE, 0, "Unexpected EOF!");
} }
if (ret == -EINVAL) { if (ret == -EINVAL) {
LogcatPanic(HELP_FALSE, "read: unexpected length.\n"); error(EXIT_FAILURE, 0, "Unexpected length.");
} }
LogcatPanic(HELP_FALSE, "logcat read failure\n"); error(EXIT_FAILURE, errno, "Logcat read failure");
} }
if (log_msg.id() > LOG_ID_MAX) { if (log_msg.id() > LOG_ID_MAX) {
LogcatPanic(HELP_FALSE, "read: unexpected log id (%d) over LOG_ID_MAX (%d)", error(EXIT_FAILURE, 0, "Unexpected log id (%d) over LOG_ID_MAX (%d).", log_msg.id(),
log_msg.id(), LOG_ID_MAX); LOG_ID_MAX);
} }
PrintDividers(log_msg.id(), printDividers); PrintDividers(log_msg.id(), printDividers);