Code to address issue148 providing both auditd.conf and userspace sol… #150
Conversation
auparse/auditd-config.c
Outdated
| void free_config(struct daemon_conf *config) | ||
| { | ||
| free((void*)config->log_file); | ||
| if (config->log_file == NULL) |
There was a problem hiding this comment.
free() is advertised as just returning if NULL is passed so that we do not need to check if its NULL before calling free. Besides, I think you meant !=, but its totally unnecessary.
There was a problem hiding this comment.
Correct, my bad. I did mean !=.
Old habits die hard .. I like to only call a memory management routine when I need to.
Do you want the if deleted and we rely upon free() to do the test?
The setting of config->log_file to be NULL after the free() is important in case we have the sequence of two calls to free_config() on the same structure.
There was a problem hiding this comment.
yes for deleting the 'if'. Setting to NULL is fine
|
|
||
| if (config.end_of_event_timeout != EOE_TIMEOUT) | ||
| eoe_timeout = (time_t)config.end_of_event_timeout; | ||
|
|
There was a problem hiding this comment.
I'm wondering if config.log_file ought to be kept so that it can be given to setup_log_file_array() if needed? Also, the if really isn't necessary since if they are the same it's harmless.
There was a problem hiding this comment.
I thought about only having one struct daemon_conf structure the same way that ausearch/aureport does, but decided on minimal change ... that would have been a bit more change given we only call setup_log_file_array for only one of a number of ausource_t types.
And yes, we can ditch the if()
There was a problem hiding this comment.
The if has been removed
src/auditd-config.c
Outdated
| @@ -1961,7 +1996,9 @@ void free_config(struct daemon_conf *config) | |||
| free((void *)config->krb5_key_file); | |||
| free((void *)config->plugin_dir); | |||
| free((void *)config_dir); | |||
| free(config_file); | |||
| if (config_file != NULL) | |||
| free(config_file); | |||
There was a problem hiding this comment.
Same comment here, free can handle NULL values.
There was a problem hiding this comment.
As before, I like to control memory management routines.
There was a problem hiding this comment.
The if has been removed.
src/ausearch-lol.c
Outdated
| @@ -35,6 +35,10 @@ | |||
| static int ready = 0; | |||
| event very_first_event; | |||
|
|
|||
| // End of Event timeout value (in seconds). This is over-ridden by auditd-conf's end_of_event_timeout configuration item | |||
| time_t eoe_timeout = EOE_TIMEOUT; | |||
There was a problem hiding this comment.
Should this be a static variable and we have a set function that ausearch/report calls unconditionally to initialize this with either the config file value, the passed value, or the default value?
There was a problem hiding this comment.
If this is the philosophy wanted, then I can do it. Please advise.
There was a problem hiding this comment.
I think it would be better to have the source of truth in one place and it gets set from outside by the app which should consider which of the three the values it is and make it so.
Didn't merge master before updating
…arch/aureport implementation
|
Looks good. Thanks! |
This PR addresses issue #148 by providing