logging_chrome: remove log file extension in the right place
An earlier commit needlessly ruined GenerateTimestampedName()
making it indiscriminately remove extensions. The extension-chopping
code is being put in the right place (at the caller site).
BUG=chromium:781363
TEST=none
Change-Id: Ic0edf8e082ce9af34b6679bec179e76e8f4797dc
Reviewed-on: https://chromium-review.googlesource.com/838242
Commit-Queue: Luigi Semenzato <[email protected]>
Reviewed-by: Lei Zhang <[email protected]>
Cr-Commit-Position: refs/heads/master@{#532113}diff --git a/chrome/common/logging_chrome.cc b/chrome/common/logging_chrome.cc
index e05e316..27e9b5c 100644
--- a/chrome/common/logging_chrome.cc
+++ b/chrome/common/logging_chrome.cc
@@ -167,14 +167,15 @@
if (symlink_path.Extension() == ".LATEST") {
base::FilePath extensionless_path = symlink_path.ReplaceExtension("");
base::FilePath target_path;
+ bool extensionless_symlink_exists =
+ base::ReadSymbolicLink(extensionless_path, &target_path);
- base::ReadSymbolicLink(extensionless_path, &target_path);
if (target_path != symlink_path) {
// No link, or wrong link. Clean up. This should happen only once in
// each log directory after the OS version update, but some of those
// directories may not be accessed for a long time, so this code needs to
// stay in forever :/
- if (base::PathExists(extensionless_path) &&
+ if (extensionless_symlink_exists &&
!base::DeleteFile(extensionless_path, false)) {
DPLOG(WARNING) << "Cannot delete " << extensionless_path.value();
}
@@ -190,29 +191,32 @@
//
// If starting a new log, then rename the old symlink as
// symlink_path.PREVIOUS and make a new symlink to a fresh log file.
- base::FilePath target_path;
- bool symlink_exists = base::PathExists(symlink_path);
- if (new_log || !symlink_exists) {
- target_path = GenerateTimestampedName(symlink_path, base::Time::Now());
- if (symlink_exists) {
- base::FilePath previous_symlink_path =
- symlink_path.ReplaceExtension(".PREVIOUS");
- // Rename symlink to .PREVIOUS. This nukes an existing symlink just like
- // the rename(2) syscall does.
- if (!base::ReplaceFile(symlink_path, previous_symlink_path, nullptr)) {
- DPLOG(WARNING) << "Cannot rename " << symlink_path.value() << " to "
- << previous_symlink_path.value();
- }
+ // Check for existence of the symlink.
+ base::FilePath target_path;
+ bool symlink_exists = base::ReadSymbolicLink(symlink_path, &target_path);
+
+ if (symlink_exists && !new_log)
+ return target_path;
+
+ // Remove any extension before time-stamping.
+ target_path = GenerateTimestampedName(symlink_path.RemoveExtension(),
+ base::Time::Now());
+
+ if (symlink_exists) {
+ base::FilePath previous_symlink_path =
+ symlink_path.ReplaceExtension(".PREVIOUS");
+ // Rename symlink to .PREVIOUS. This nukes an existing symlink just like
+ // the rename(2) syscall does.
+ if (!base::ReplaceFile(symlink_path, previous_symlink_path, nullptr)) {
+ DPLOG(WARNING) << "Cannot rename " << symlink_path.value() << " to "
+ << previous_symlink_path.value();
}
- // If all went well, the symlink no longer exists. Recreate it.
- if (!base::CreateSymbolicLink(target_path, symlink_path)) {
- DPLOG(ERROR) << "Unable to create symlink " << symlink_path.value()
- << " pointing at " << target_path.value();
- }
- } else {
- if (!base::ReadSymbolicLink(symlink_path, &target_path))
- DPLOG(ERROR) << "Unable to read symlink " << symlink_path.value();
+ }
+ // If all went well, the symlink no longer exists. Recreate it.
+ if (!base::CreateSymbolicLink(target_path, symlink_path)) {
+ DPLOG(ERROR) << "Unable to create symlink " << symlink_path.value()
+ << " pointing at " << target_path.value();
}
return target_path;
}
@@ -256,7 +260,7 @@
.Append(GetLogFileName(command_line).BaseName());
}
-#endif // OS_CHROMEOS
+#endif // defined(OS_CHROMEOS)
void InitChromeLogging(const base::CommandLine& command_line,
OldFileDeletionState delete_old_log_file) {
@@ -291,7 +295,7 @@
// the link, it shouldn't remove the old file in the logging code,
// since that will remove the newly created link instead.
delete_old_log_file = APPEND_TO_OLD_LOG_FILE;
-#endif
+#endif // defined(OS_CHROMEOS)
} else {
log_locking_state = DONT_LOCK_LOG_FILE;
}
@@ -311,13 +315,13 @@
chrome_logging_failed_ = true;
return;
}
-#else
+#else // defined(OS_CHROMEOS)
if (!success) {
DPLOG(ERROR) << "Unable to initialize logging to " << log_path.value();
chrome_logging_failed_ = true;
return;
}
-#endif
+#endif // defined(OS_CHROMEOS)
// We call running in unattended mode "headless", and allow headless mode to
// be configured either by the Environment Variable or by the Command Line
@@ -417,11 +421,6 @@
base::Time timestamp) {
base::Time::Exploded time_deets;
timestamp.LocalExplode(&time_deets);
- base::FilePath new_path = base_path;
- // Assume that the base_path is "chrome.LATEST", and remove the extension.
- // Ideally we would also check the value of base_path, but we cannot reliably
- // log anything here, and aborting seems too harsh a choice.
- new_path = new_path.ReplaceExtension("");
std::string suffix = base::StringPrintf("_%02d%02d%02d-%02d%02d%02d",
time_deets.year,
time_deets.month,
@@ -429,7 +428,7 @@
time_deets.hour,
time_deets.minute,
time_deets.second);
- return new_path.InsertBeforeExtensionASCII(suffix);
+ return base_path.InsertBeforeExtensionASCII(suffix);
}
#endif // defined(OS_CHROMEOS)
