mac: Use os_log() when the deployment target is 10.12 or hig... [chromium/mini_chromium : master]

508 views
Skip to first unread message

Mark Mentovai (Gerrit)

unread,
Aug 4, 2016, 1:29:18 PM8/4/16
to Robert Sesek, crashp...@chromium.org
Mark Mentovai has posted comments on this change. (
https://chromium-review.googlesource.com/366371 )

Change subject: mac: Use os_log() when the deployment target is 10.12 or
higher
......................................................................


Patch Set 1:

I’m doing this in mini_chromium first because it might actually build with
a 10.12 deployment target. It normally uses the system’s default deployment
target, which will be 10.12 when building on 10.12 with an Xcode 8 beta.
Chromium always sets its own deployment target, and it’s always below
10.12, so we wouldn’t see deprecation warnings there yet.

--
To view, visit https://chromium-review.googlesource.com/366371
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd65b181d495451aeb3ad7dd6a130a1f65544253
Gerrit-PatchSet: 1
Gerrit-Project: chromium/mini_chromium
Gerrit-Branch: master
Gerrit-Owner: Mark Mentovai <ma...@chromium.org>
Gerrit-Reviewer: Mark Mentovai <ma...@chromium.org>
Gerrit-Reviewer: Robert Sesek <rse...@chromium.org>
Gerrit-HasComments: No

Robert Sesek (Gerrit)

unread,
Aug 4, 2016, 4:09:50 PM8/4/16
to Mark Mentovai, Robert Sesek, crashp...@chromium.org
Robert Sesek has posted comments on this change. (
https://chromium-review.googlesource.com/366371 )

Change subject: mac: Use Unified Logging when the deployment target is
10.12 or higher
......................................................................


Patch Set 3: Code-Review+1

--
To view, visit https://chromium-review.googlesource.com/366371
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd65b181d495451aeb3ad7dd6a130a1f65544253
Gerrit-PatchSet: 3

Mark Mentovai

unread,
Aug 4, 2016, 5:10:17 PM8/4/16
to Robert Sesek, Crashpad-dev
In the e-mail that this generated, it looks like you changed the subject.

I had edited the commit message (including the first line), which might have changed the subject. If you also made a change to the subject, then that’s cool, nothing to see here, move along. But if it’s taking my subject change and attributing it to you, then we’ve got a Gerrit problem that we should get on file with them.

Your move.

Robert Sesek

unread,
Aug 4, 2016, 5:11:28 PM8/4/16
to Mark Mentovai, Crashpad-dev
I didn't change the subject intentionally...

rsesek / @chromium.org

On Thu, Aug 4, 2016 at 5:10 PM, Mark Mentovai <ma...@chromium.org> wrote:
In the e-mail that this generated, it looks like you changed the subject.

I had edited the commit message (including the first line), which might have changed the subject. If you also made a change to the subject, then that’s cool, nothing to see here, move along. But if it’s taking my subject change and attributing it to you, then we’ve got a Gerrit problem that we should get on file with them.

Your move.

Mark Mentovai

unread,
Aug 4, 2016, 5:17:15 PM8/4/16
to Robert Sesek, Crashpad-dev, Andrew Bonventre
Oh, hmm, I think it’s just confusing wording, actually. The e-mail is saying “Change subject” as in “this is the subject of the change.” I was reading it as “Robert changed the subject”. Cc andybons as an overall Gerrit usability thing.

Mark Mentovai (Gerrit)

unread,
Aug 5, 2016, 11:24:50 AM8/5/16
to Robert Sesek, crashp...@chromium.org
Mark Mentovai has submitted this change and it was merged. (
https://chromium-review.googlesource.com/366371 )

Change subject: mac: Use Unified Logging when the deployment target is
10.12 or higher
......................................................................


mac: Use Unified Logging when the deployment target is 10.12 or higher

Unified Logging (os_log) replaces ASL as the preferred system logging
interface in 10.12. It’s not available prior to 10.12, so ASL is still
used there. Attempting to use ASL with the deployment target set to
10.12 or higher produces deprecation warnings.

https://developer.apple.com/reference/os/1891852-logging

BUG=crashpad:120

Change-Id: Ibd65b181d495451aeb3ad7dd6a130a1f65544253
Reviewed-on: https://chromium-review.googlesource.com/366371
Reviewed-by: Robert Sesek <rse...@chromium.org>
---
M base/logging.cc
1 file changed, 59 insertions(+), 13 deletions(-)

Approvals:
Robert Sesek: Looks good to me



diff --git a/base/logging.cc b/base/logging.cc
index 224581b..0f19b15 100644
--- a/base/logging.cc
+++ b/base/logging.cc
@@ -18,9 +18,15 @@
#endif // OS_POSIX

#if defined(OS_MACOSX)
-#include <asl.h>
+#include <AvailabilityMacros.h>
#include <CoreFoundation/CoreFoundation.h>
#include <pthread.h>
+#if !defined(MAC_OS_X_VERSION_10_12) || \
+ MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_12
+#include <asl.h>
+#else
+#include <os/log.h>
+#endif
#elif defined(OS_LINUX)
#include <sys/syscall.h>
#include <sys/types.h>
@@ -118,7 +124,7 @@
fflush(stderr);

#if defined(OS_MACOSX)
- const bool log_via_asl = []() {
+ const bool log_to_system = []() {
struct stat stderr_stat;
if (fstat(fileno(stderr), &stderr_stat) == -1) {
return true;
@@ -136,18 +142,18 @@
stderr_stat.st_rdev == dev_null_stat.st_rdev;
}();

- if (log_via_asl) {
+ if (log_to_system) {
CFBundleRef main_bundle = CFBundleGetMainBundle();
CFStringRef main_bundle_id_cf =
main_bundle ? CFBundleGetIdentifier(main_bundle) : nullptr;

std::string main_bundle_id_buf;
- const char* asl_facility = nullptr;
+ const char* main_bundle_id = nullptr;

if (main_bundle_id_cf) {
- asl_facility =
+ main_bundle_id =
CFStringGetCStringPtr(main_bundle_id_cf, kCFStringEncodingUTF8);
- if (!asl_facility) {
+ if (!main_bundle_id) {
// 1024 is from 10.10.5 CF-1153.18/CFBundle.c __CFBundleMainID__
(at
// the point of use, not declaration).
main_bundle_id_buf.resize(1024);
@@ -157,16 +163,17 @@
kCFStringEncodingUTF8)) {
main_bundle_id_buf.clear();
} else {
- asl_facility = &main_bundle_id_buf[0];
+ main_bundle_id = &main_bundle_id_buf[0];
}
}
}

- if (!asl_facility) {
- asl_facility = "com.apple.console";
- }
+#if !defined(MAC_OS_X_VERSION_10_12) || \
+ MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_12
+ // Use ASL when this might run on pre-10.12 systems. Unified Logging
+ // (os_log) was introduced in 10.12.

- class ASLClient {
+ const class ASLClient {
public:
explicit ASLClient(const char* asl_facility)
: client_(asl_open(nullptr, asl_facility, ASL_OPT_NO_DELAY)) {}
@@ -177,9 +184,9 @@
private:
aslclient client_;
DISALLOW_COPY_AND_ASSIGN(ASLClient);
- } asl_client(asl_facility);
+ } asl_client(main_bundle_id ? main_bundle_id : "com.apple.console");

- class ASLMessage {
+ const class ASLMessage {
public:
ASLMessage() : message_(asl_new(ASL_TYPE_MSG)) {}
~ASLMessage() { asl_free(message_); }
@@ -222,6 +229,45 @@
asl_set(asl_message.get(), ASL_KEY_MSG, str_newline.c_str());

asl_send(asl_client.get(), asl_message.get());
+#else
+ // Use Unified Logging (os_log) when this will only run on 10.12 and
later.
+ // ASL is deprecated in 10.12.
+
+ const class OSLog {
+ public:
+ explicit OSLog(const char* subsystem)
+ : os_log_(subsystem ?
os_log_create(subsystem, "chromium_logging")
+ : OS_LOG_DEFAULT) {}
+ ~OSLog() {
+ if (os_log_ != OS_LOG_DEFAULT) {
+ os_release(os_log_);
+ }
+ }
+
+ os_log_t get() const { return os_log_; }
+
+ private:
+ os_log_t os_log_;
+ DISALLOW_COPY_AND_ASSIGN(OSLog);
+ } log(main_bundle_id);
+
+ const os_log_type_t os_log_type = [](LogSeverity severity) {
+ switch (severity) {
+ case LOG_INFO:
+ return OS_LOG_TYPE_INFO;
+ case LOG_WARNING:
+ return OS_LOG_TYPE_DEFAULT;
+ case LOG_ERROR:
+ return OS_LOG_TYPE_ERROR;
+ case LOG_FATAL:
+ return OS_LOG_TYPE_FAULT;
+ default:
+ return severity < 0 ? OS_LOG_TYPE_DEBUG : OS_LOG_TYPE_DEFAULT;
+ }
+ }(severity_);
+
+ os_log_with_type(log.get(), os_log_type, "%{public}s",
str_newline.c_str());
+#endif
}
#elif defined(OS_WIN)
OutputDebugString(base::UTF8ToUTF16(str_newline).c_str());
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibd65b181d495451aeb3ad7dd6a130a1f65544253
Gerrit-PatchSet: 5
Reply all
Reply to author
Forward
0 new messages