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.

Sign up for GitHub

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

feat(logging): setup min log level, log dir and file mode #328

Merged
merged 1 commit into from
Dec 17, 2019
Merged

feat(logging): setup min log level, log dir and file mode #328

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/rime/setup.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,20 @@ RIME_API void LoadModules(const char* module_names[]) {
}
}

RIME_API void SetupLogging(const char* app_name) {
RIME_API void SetupLogging(const char* app_name, int min_log_level, const char* log_dir) {
#ifdef RIME_ENABLE_LOGGING
FLAGS_minloglevel = min_log_level;
if (log_dir) {
FLAGS_log_dir = log_dir;
}
// Do not allow other users to read/write log files created by current process.
FLAGS_logfile_mode = 0600;
google::InitGoogleLogging(app_name);
#endif // RIME_ENABLE_LOGGING
}

RIME_API void SetupLogging(const char* app_name) {
SetupLogging(app_name, 0, NULL);
}

} // namespace rime
1 change: 1 addition & 0 deletions src/rime/setup.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ extern const char* kLegacyModules[];

RIME_API void LoadModules(const char* module_names[]);

RIME_API void SetupLogging(const char* app_name, int min_log_level, const char* log_dir);
RIME_API void SetupLogging(const char* app_name);

} // namespace rime
Expand Down
2 changes: 1 addition & 1 deletion src/rime_api.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ RIME_API void RimeSetup(RimeTraits *traits) {

setup_deployer(traits);
if (PROVIDED(traits, app_name)) {
SetupLogging(traits->app_name);
SetupLogging(traits->app_name, traits->min_log_level, traits->log_dir);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I think this line introduce an ABI breakage. See hchunhui/librime-lua#25 (comment) .

Clients built with older librime will pass a traits without min_log_level and log_dir at runtime, so we may want to check data_size before we use the two new members.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hchunhui Thanks for the report. Fixed in 62bbead

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think that 62bbead is going to work as expected.
PROVIDED is defined as:

// assuming member is a pointer in struct *p
#define PROVIDED(p, member) ((p) && RIME_STRUCT_HAS_MEMBER(*(p), (p)->member) && (p)->member)

log_dir is a pointer (const char*), so PROVIDED(traits, log_dir) might work, but min_log_level is of type int and value 0 is one of several valid values of this variable, so PROVIDED(traits, min_log_level) could return false negative.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed. thanks.

}
}

Expand Down
10 changes: 10 additions & 0 deletions src/rime_api.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,16 @@ typedef struct rime_traits_t {

//! A list of modules to load before initializing
const char** modules;
// v1.6
/*! Minimal level of logged messages.
* Value is passed to Glog library using FLAGS_minloglevel variable.
* 0 = INFO (default), 1 = WARNING, 2 = ERROR, 3 = FATAL
*/
int min_log_level;
/*! Directory of log files.
* Value is passed to Glog library using FLAGS_log_dir variable.
*/
const char* log_dir;
} RimeTraits;

typedef struct {
Expand Down