-
Notifications
You must be signed in to change notification settings - Fork 10
Introduce LOG_LEVEL build flag to memcr.c #114
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
Conversation
ef6a231 to
4855695
Compare
|
The logging part of this change looks good. I have a refactor of this on my TODO list but haven't gotten to it yet. Alternatively, you could consider reconfiguring your log collector to discard old entries, but that's a separate discussion. I'm not sure why you added casting changes in the same commit - I'd prefer them to be in a separate change with some explanation of why the casting is needed. Did you spot a bug? Compiler warnings? If so - which platform/compiler? I don't see any problems related to this and no warnings on any platform/compiler I test on - and I test on all supported architectures. |
memcr.c
Outdated
|
|
||
| while (1) { | ||
| ret = read(fd, buf + off, count - off); | ||
| ret = read(fd, (char *)buf + off, count - off); |
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.
why?
memcr.c
Outdated
| } | ||
|
|
||
| if (ret < count - off) { | ||
| if ((size_t)ret < count - off) { |
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.
why?
memcr.c
Outdated
|
|
||
| while (1) { | ||
| ret = write(fd, buf + off, count - off); | ||
| ret = write(fd, (const char *)buf + off, count - off); |
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.
why?
memcr.c
Outdated
| } | ||
|
|
||
| if (ret < count - off) { | ||
| if ((size_t)ret < count - off) { |
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.
why?
| struct vm_area *vma = &vmas[idx]; | ||
|
|
||
| fprintf(stdout, "%d\t%lx-%lx %c%c%c\t%ld bytes\t(%ld kB)\n", idx, vma->start, vma->end, vma->prot & PROT_READ ? 'r' : '-', vma->prot & PROT_WRITE ? 'w' : '-', vma->prot & PROT_EXEC ? 'x' : '-', vma->end - vma->start, (vma->end - vma->start) / 1024); | ||
| } |
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'd prefer to keep the code under #if 0 for now - it was useful during development - I might remove it later or make it runtime configurable.
| { "version", 0, NULL, 'V'}, | ||
| { NULL, 0, NULL, 0 } | ||
| { NULL, 0, NULL, 0 } | ||
| }; |
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.
now the closing bracket is unaligned
memcr.c
Outdated
| print_version(); | ||
|
|
||
| page_size = getpagesize(); | ||
| page_size = (unsigned int)getpagesize(); |
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.
why?
memcr.c
Outdated
| ret = service_mode(listen_location, listen_gid); | ||
| else | ||
| ret = user_interactive_mode(pid); | ||
| ret = user_interactive_mode((pid_t)pid); |
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.
why?
memcr.c
Outdated
| { | ||
| ssize_t ret; | ||
| const char *msg = "[i] SIGINT\n"; | ||
| const char *msg_str = "[i] SIGINT\n"; |
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.
this is inconsistent with the rest of the code where we don't add _str suffix
memcr.c
Outdated
| fprintf(stderr, "[-] munmap blob failed: %ld\n", ret); | ||
| return ret; | ||
| err("munmap blob failed: %ld\n", ret); | ||
| return (int)ret; |
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.
why?
4855695 to
d2c7632
Compare
d2c7632 to
f1fbeb5
Compare
Introduces Log Levels in memcr.c:
err(...) - level 0
mil(...) - level 1
log(...) - level 2
msg(...) - level 3
To be able to reduce amount of memcr logs for release/prod builds.