Skip to content
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

mvLog() is double-spacing; how use mvLog() in regards to trailing newline? #64

Open
diablodale opened this issue May 21, 2023 · 2 comments

Comments

@diablodale
Copy link
Contributor

XLink double-spaces the mvLog output often. Nothing breaks, but when the log volume is large and multiple things are logging, the doublespacing doesn't help. 😉

While writing XLink code, I see inconsistencies of mvLog() format strings. Some have a trailing \n and some do not.
On Windows and Linux, this trailing newline in the format string causes doublespaced log output on the console.

This can be resolved by choosing a policy, enforcing it, and then adjusting the mvLog calls to follow that policy. Bulk changes are easy with tools like vscode its regex search/replace.

Setup

  • XLink at develop 457b23f
  • Windows and Linux. Maybe other platforms
  • MSVC v16.11.26 and g++ v11.3.0

Repo

Found by watching logs and then mvLog code review. Easy repro is to...

  1. add the following code at the top of the function XLinkPlatformFindDevices
mvLog(MVLOG_FATAL, "Test line 1\n");
mvLog(MVLOG_FATAL, "Test line 2\n");
mvLog(MVLOG_FATAL, "Test line 3\n");
  1. config, build for Debug
  2. set env var XLINK_LEVEL=fatal the value must be lowercase
  3. run the test examples/list_devices

Result

Both Windows and Linux have double-spaced log entries

F: [global] [         0] [ThreadN] XLinkPlatformFindDevices:58  Test line 1

F: [global] [         0] [ThreadN] XLinkPlatformFindDevices:59  Test line 2

F: [global] [         0] [ThreadN] XLinkPlatformFindDevices:60  Test line 3

Expected

F: [global] [         0] [ThreadN] XLinkPlatformFindDevices:58  Test line 1
F: [global] [         0] [ThreadN] XLinkPlatformFindDevices:59  Test line 2
F: [global] [         0] [ThreadN] XLinkPlatformFindDevices:60  Test line 3

Notes

This is due to use of \n in the format string. When code calls mvLog(MVLOG_FATAL, "Test line 1\n"); with a trailing space, this causes double-spaced entries due to a trailing newline also added by mvLog() itself. There are 4 platform scenarios. 3/4 adds a newline. 1/4 (Android) doesn't.

XLink/src/shared/XLinkLog.c

Lines 162 to 186 in 457b23f

#ifdef __ANDROID__
// Convert to Android logging enumeration
enum android_LogPriority logPrio = ANDROID_LOG_DEBUG + (lvl - MVLOG_DEBUG);
//__android_log_print(logPrio, UNIT_NAME_STR, headerFormat, mvLogHeader[lvl], UNIT_NAME_STR, timestamp, threadName, func, line);
__android_log_vprint(logPrio, UNIT_NAME_STR, format, args);
// __android_log_print(logPrio, UNIT_NAME_STR, "%s", ANSI_COLOR_RESET);
#else
fprintf(stdout, headerFormat, mvLogHeader[lvl], UNIT_NAME_STR, timestamp, threadName, func, line);
vfprintf(stdout, format, args);
fprintf(stdout, "%s\n", ANSI_COLOR_RESET);
#endif
#elif defined __shave__
printf(headerFormat, mvLogHeader[lvl], UNIT_NAME_STR, timestamp, threadName, func, line);
printf(format, args);
printf("%s\n", ANSI_COLOR_RESET);
#endif
#ifdef __RTEMS__
}
else
{
printk(headerFormat, mvLogHeader[lvl], UNIT_NAME_STR, timestamp, threadName, func, line);
vprintk(format, args);
printk("%s\n", ANSI_COLOR_RESET);

@themarpe
Copy link
Collaborator

+1 on this - I'd go with mvLog adding the trailing newline itself and then removing the explicit \n in the calls, as this is how other libraries handle it usually as well

diablodale added a commit to diablodale/XLink that referenced this issue May 29, 2023
@diablodale
Copy link
Contributor Author

The fix is in my fork, github linked to it above. Should be an easy cherry-pick; usb_host.cpp might not merge automatically.

I used vscode regex search/replace of:

mvLog\((.+)\\n"(.+)
mvLog($1"$2

then hand edited 4 in src\shared\XLinkDispatcherImpl.c due to their calls being multiline.
then verified diff by eye. I found a few that had trailing spaces that I manually removed.

I also saw raw printf() and perror() calls in tcpip_host.cpp and XLinkDispatcher.c files, DEBUG() macro, functions like XLinkProfPrint(), etc. The OP issue and the linked fix do not address these. The printf output may be poorly interlaced with mvLog() output since they are on a different output buffer than mvLog().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants