Don't explicitly use files for mutex creation#3285
Don't explicitly use files for mutex creation#3285marcstern wants to merge 3 commits intoowasp-modsecurity:v2/masterfrom
Conversation
We don't have to generate a temp name ourselves, it'll be done in apr_global_mutex_create(). We don't have to provide a filename, apr_global_mutex_create() generates one automatically. Moreover, under Unix & Windows, the preferred mechanism won't use a file at all. apr_file_mktemp() cannot be used as it creates the file (at least on FreeBSD). Discussion in Apache mailing list: https://lists.apache.org/thread/ykb26kg4lgcqnldvxwd9p6hv16fy4z9l
We don't have to generate a temp name ourselves, it'll be done in apr_global_mutex_create(). We don't have to provide a filename, apr_global_mutex_create() generates one automatically. Moreover, under Unix & Windows, the preferred mechanism won't use a file at all. apr_file_mktemp() cannot be used as it creates the file (at least on FreeBSD). Discussion in Apache mailing list: https://lists.apache.org/thread/ykb26kg4lgcqnldvxwd9p6hv16fy4z9l
|
@marcstern ping. |
|
fzipi
left a comment
fzipi
left a comment
There was a problem hiding this comment.
There is no better feeling than removing unneeded code.
| rc = apr_global_mutex_create(lock, filename, APR_LOCK_DEFAULT, mp); | ||
| apr_status_t rc = apr_global_mutex_create(lock, NULL, APR_LOCK_DEFAULT, mp); | ||
| if (rc != APR_SUCCESS) { | ||
| ap_log_perror(APLOG_MARK, APLOG_ERR, 0, mp, " ModSecurity: Could not create global mutex"); |
There was a problem hiding this comment.
Passing rc as the apr_status_t argument rather than 0 would be better.
|
could you help us to review this PR and finish it (if it's necessary to do anything). Thanks! |
|
I can't push to this PR branch but I'd add: and merge this |
Ah, you're right - thanks. Probably we should open a new PR, based on this one and then odd yours. I can do that later or feel free to do it. Thanks anyway. |
|
Seems like this PR is orphan, I mean the modifications are the part of an another PR (#3269) and already merged. Closing as the issue is solved and PR is abandoned. |
4 participants
We don't have to generate a temp name ourselves, it'll be done in apr_global_mutex_create().
We don't have to provide a filename, apr_global_mutex_create() generates one automatically.
Moreover, under Unix & Windows, the preferred mechanism won't use a file at all.
apr_file_mktemp() cannot be used as it creates the file (at least on FreeBSD).
Discussion in Apache mailing list: https://lists.apache.org/thread/ykb26kg4lgcqnldvxwd9p6hv16fy4z9l