-
Notifications
You must be signed in to change notification settings - Fork 451
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mac: prjfs-log improvements #158
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd"> | ||
<plist version="1.0"> | ||
<dict> | ||
<key>IDEWorkspaceSharedSettings_AutocreateContextsIfNeeded</key> | ||
<false/> | ||
</dict> | ||
</plist> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,7 @@ | |
#include <CoreFoundation/CoreFoundation.h> | ||
#include <IOKit/IOKitLib.h> | ||
#include <mach/mach_time.h> | ||
|
||
#include <sys/time.h> | ||
|
||
static const char* KextLogLevelAsString(KextLog_Level level); | ||
|
||
|
@@ -24,7 +24,9 @@ int main(int argc, const char * argv[]) | |
std::cerr << "Failed to set up shared data queue.\n"; | ||
return 1; | ||
} | ||
|
||
|
||
__block int lineCount = 0; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not Objective-C per se. Apple's C++ compiler has an extension to support blocks, which is how we can call the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ahh okay thanks! I missed that |
||
|
||
dispatch_source_set_event_handler(dataQueue.dispatchSource, ^{ | ||
struct { | ||
mach_msg_header_t msgHdr; | ||
|
@@ -39,15 +41,23 @@ int main(int argc, const char * argv[]) | |
{ | ||
break; | ||
} | ||
|
||
int messageSize = entry->size; | ||
if (messageSize >= sizeof(KextLog_MessageHeader) + 2) | ||
{ | ||
struct KextLog_MessageHeader message = {}; | ||
memcpy(&message, entry->data, sizeof(KextLog_MessageHeader)); | ||
const char* messageType = KextLogLevelAsString(message.level); | ||
int logStringLength = messageSize - sizeof(KextLog_MessageHeader) - 1; | ||
printf("%s: %.*s\n", messageType, logStringLength, entry->data + sizeof(KextLog_MessageHeader)); | ||
|
||
struct timeval tp; | ||
gettimeofday(&tp, NULL); | ||
long int timestamp = tp.tv_sec * 1000 + tp.tv_usec / 1000; | ||
|
||
printf("(%d: %ld) %s: %.*s\n", lineCount, timestamp, messageType, logStringLength, entry->data + sizeof(KextLog_MessageHeader)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @sanoursa, what was your reason for adding line number to the output? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It just made it easier for me to compare different test runs and compare logs. I added the timestamp later, you could definitely argue that we don't need both. What are your thoughts? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had been thinking we wouldn't need line numbers (text editors can provide that) but I can seen the benefit now to keeping the line numbers because we expect the log message to contain new lines. I think we can keep the line numbers in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I can see the value in keeping the line numbers. I vote to keep them in too 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now that you mention it, I think that (multiline traces) is why I added it :-). Hard to remember to the way back from before vacation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For what it's worth, there is also a timestamp field in the message itself. This is in units of Mach Absolute Time, which is a monotonically increasing clock as opposed to one linked to system time. This time stamp of course also reflects the time when the kernel generated the log message, not when it arrived in the logging daemon. Given we're just outputting seconds here, using the Mach timestamp may be closer to what we want, although I'm of course not exactly sure what it is you do want. One major difference would be that it's not in seconds since midnight, but (nano-)seconds since some arbitrary time, such as the time when the logging daemon started. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah it probably makes sense to use that then. I just wanted to be able to see relative timings of events. I'll clean that up before merging. |
||
lineCount++; | ||
} | ||
|
||
IODataQueueDequeue(dataQueue.queueMemory, nullptr, nullptr); | ||
} | ||
}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How messy would the output be to separate these with
|
rather than newlines?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depends on the event. Sometimes we'll see 3-4 actions in a single kauth event. I'm not opposed to reformatting this though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm good with going what you have now and we can see how it feels in practice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep feel free to clean it up if the multiple lines gets to be too much