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

log/eve: Threaded filename change: eve.N.json #5461

Closed
wants to merge 4 commits into from

Conversation

jlucovsky
Copy link
Contributor

Continuation of #5454

This commit changes the name of the file used with threaded eve logging
to better support log rotation

Instead of using "eve.json.N" and creating potential issues with log
rotation (which also uses a ".N" suffix), the eve logs will be named
"eve.N.json" when threaded.

Describe changes:

  • Remove extra SCLogNotice calls

PRScript output (if applicable):

#suricata-verify-pr:
#suricata-verify-repo:
#suricata-verify-branch:
#suricata-update-pr:
#suricata-update-repo:
#suricata-update-branch:
#libhtp-pr:
#libhtp-repo:
#libhtp-branch:

This commit returns the basename of a file, if it exists
in the same way that `basename(1)` works.
This commit changes the name of the file used with threaded eve logging
to better support log rotation

Instead of using "eve.json.N" and creating potential issues with log
rotation (which also uses a ".N" suffix), the eve logs will be named
"eve.N.json" when threaded.
snprintf(fname, sizeof(fname), "%s.%d", log_path, SC_ATOMIC_ADD(eve_file_suffix, 1));
if (!LogFileThreadedName(log_path, fname, sizeof(fname), SC_ATOMIC_ADD(eve_file_id, 1))) {
SCLogError(SC_ERR_MEM_ALLOC, "Unable to create threaded filename for log");
return false;
Copy link
Member

Choose a reason for hiding this comment

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

thread is leaked here. Should this be a goto error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes -- good catch; thanks.

@jlucovsky
Copy link
Contributor Author

Continued in #5463

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

Successfully merging this pull request may close these issues.

2 participants