-
Notifications
You must be signed in to change notification settings - Fork 510
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
common: no out_*() functions in release build #6026
Conversation
2df9c5f
to
72eb99a
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6026 +/- ##
=======================================
Coverage 70.04% 70.05%
=======================================
Files 133 133
Lines 19564 19563 -1
Branches 3263 3263
=======================================
Hits 13704 13704
+ Misses 5860 5859 -1 |
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.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @grom72)
a discussion (no related file):
Please also:
- remove
out_log_va()
from theout.h
header and - wrap the
out_log()
declaration in theout.h
header into#ifdef DEBUG
just to make things clear.
src/core/out.c
line 164 at r1 (raw file):
CORE_LOG_HARK("pid %d: program: %s", getpid(), util_getexecname(namepath, PATH_MAX)); #endif /* DEBUG */
Suggestion:
static char namepath[PATH_MAX];
CORE_LOG_HARK("pid %d: program: %s", getpid(),
util_getexecname(namepath, PATH_MAX));
src/core/out.c
line 193 at r1 (raw file):
CORE_LOG_HARK("%s", drd_msg); #endif /* VG_DRD_ENABLED */ #endif
Suggestion:
#endif /* DEBUG */
src/core/out.c
line 309 at r1 (raw file):
void out_log_va(const char *file, int line, const char *func, int level, const char *fmt, va_list ap)
Suggestion:
static void
out_log_va(const char *file, int line, const char *func, int level,
const char *fmt, va_list ap)
72eb99a
to
95ed5f5
Compare
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.
Reviewable status: 0 of 2 files reviewed, 4 unresolved discussions (waiting on @janekmi)
a discussion (no related file):
Previously, janekmi (Jan Michalski) wrote…
Please also:
- remove
out_log_va()
from theout.h
header and- wrap the
out_log()
declaration in theout.h
header into#ifdef DEBUG
just to make things clear.
Done.
src/core/out.c
line 164 at r1 (raw file):
CORE_LOG_HARK("pid %d: program: %s", getpid(), util_getexecname(namepath, PATH_MAX)); #endif /* DEBUG */
Done.
src/core/out.c
line 193 at r1 (raw file):
CORE_LOG_HARK("%s", drd_msg); #endif /* VG_DRD_ENABLED */ #endif
Done.
src/core/out.c
line 309 at r1 (raw file):
void out_log_va(const char *file, int line, const char *func, int level, const char *fmt, va_list ap)
Done.
95ed5f5
to
eacaa9b
Compare
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.
Reviewable status: 0 of 2 files reviewed, 4 unresolved discussions (waiting on @janekmi)
a discussion (no related file):
Previously, grom72 (Tomasz Gromadzki) wrote…
Done.
Small fix:
can not wrap out_log()
as it is used in ut_log_function
:(
Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@intel.com>
eacaa9b
to
2129693
Compare
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.
Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @janekmi)
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.
Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @grom72)
a discussion (no related file):
can not wrap
out_log()
as it is used inut_log_function
:(
How can it be without DEBUG
if you had just made it available only if DEBUG
is set?
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.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @janekmi)
a discussion (no related file):
Previously, janekmi (Jan Michalski) wrote…
can not wrap
out_log()
as it is used inut_log_function
:(How can it be without
DEBUG
if you had just made it available only ifDEBUG
is set?
ut_log_function() is used in debug and non debug builds
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.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @grom72)
Requires:
This change is