-
Notifications
You must be signed in to change notification settings - Fork 863
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
Adding a logging system for haicrypt #359
Conversation
haicrypt/haicrypt_log.cpp
Outdated
|
||
#include "hcrypt.h" | ||
#include "../srtcore/srt.h" | ||
#include "../srtcore/logging.h" |
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 adds another logging API wrapper on top of already complicated logging API wrapper. Can we please stick to single logging API ("srtcore/logging.h")? If we must have "Templates made C way" variation that should probably go into "srtcore/logging.h" as well.
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.
The purpose of this wrapper is to fill the existing uses of logging instructions in haicrypt with the implementation of the SRT logging system. Haicrypt, as a C project, can't use the C++ logging API from SRT directly, this wrapper is for only that sole purpose. This wrapper isn't predicted to serve for anything else, that's why the wrapper is implemented in haicrypt project, not in srtcore.
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.
Having C-style logging API available throughout the project is a good thing and actually has been asked for in the past. Also, there are benefits of having all logging functionality in one place: it makes it clearer why the "C template" done the way it is and if we want to improve/change logging implementation at a later time we will only have one place to work on. Please move this code to srtcore.
haicrypt/hcrypt.c
Outdated
(void)logfa; | ||
(void)level; | ||
return 0; | ||
static char buf[4096]; |
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.
Please avoid returning non-const static buffers from a function. There is really no need for this: just pass destination buffer and its length as arguments.
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.
No description provided.