This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
pespin gerrit-no-reply at lists.osmocom.orgpespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/26089 ) Change subject: logging: Fix double lock of log_tgt_mutex ...................................................................... logging: Fix double lock of log_tgt_mutex Recent commit introduced the "blocking-io" param to "log stderr" VTY command, which calls log_target_file_switch_to_{stream,wqueue}. The VTY command already locks the log_tgt_mutex mutex, since it has to access the tgt list. However, the functions mention above also want to lock the same mutex in order to log information. Let's drop the logging to avoid the double lock, and update its documentation to mention it must be called with the lock already held, as documented on other similar functions. Fixes: b72867f0e68c96ca25e1f9929ce92be0a802db6b Related: OS#4311 Change-Id: Idb4215fa2f364e28c0bb73fb9975b6c9f50a46f6 --- M src/logging.c 1 file changed, 6 insertions(+), 36 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/89/26089/1 diff --git a/src/logging.c b/src/logging.c index 00d75c3..9e2f5c2 100644 --- a/src/logging.c +++ b/src/logging.c @@ -1090,34 +1090,21 @@ /*! switch from non-blocking/write-queue to blocking + buffered stream output * \param[in] target log target which we should switch - * \return 0 on success; 1 if already switched before; negative on error */ + * \return 0 on success; 1 if already switched before; negative on error + * Must be called with mutex osmo_log_tgt_mutex held, see log_tgt_mutex_lock. + */ int log_target_file_switch_to_stream(struct log_target *target) { struct osmo_wqueue *wq; - const char *name; if (!target) return -ENODEV; - /* this only works for file/stderr targets */ - switch (target->type) { - case LOG_TGT_TYPE_FILE: - name = target->tgt_file.fname; - break; - case LOG_TGT_TYPE_STDERR: - name = "stderr"; - break; - default: - return -EINVAL; - } - if (target->tgt_file.out) { /* target has already been switched over */ return 1; } - LOGP(DLGLOBAL, LOGL_INFO, "Switching log target '%s' to blocking stream I/O\n", name); - wq = target->tgt_file.wqueue; OSMO_ASSERT(wq); @@ -1127,8 +1114,6 @@ else target->tgt_file.out = fopen(target->tgt_file.fname, "a"); if (!target->tgt_file.out) { - LOGP(DLGLOBAL, LOGL_ERROR, "Cannot open log target '%s' as blocking stream I/O: %s\n", - name, strerror(errno)); return -EIO; } @@ -1156,35 +1141,22 @@ /*! switch from blocking + buffered file output to non-blocking write-queue based output. * \param[in] target log target which we should switch - * \return 0 on success; 1 if already switched before; negative on error */ + * \return 0 on success; 1 if already switched before; negative on error + * Must be called with mutex osmo_log_tgt_mutex held, see log_tgt_mutex_lock. + */ int log_target_file_switch_to_wqueue(struct log_target *target) { struct osmo_wqueue *wq; - const char *name; int rc; if (!target) return -ENODEV; - /* this only works for file/stderr targets */ - switch (target->type) { - case LOG_TGT_TYPE_FILE: - name = target->tgt_file.fname; - break; - case LOG_TGT_TYPE_STDERR: - name = "stderr"; - break; - default: - return -EINVAL; - } - if (!target->tgt_file.out) { /* target has already been switched over */ return 1; } - LOGP(DLGLOBAL, LOGL_INFO, "Switching log target '%s' to non-blocking I/O\n", name); - /* we create a ~640kB sized talloc pool within the write-queue to ensure individual * log lines (stored as msgbs) will not put result in malloc() calls, and also to * reduce the OOM probability within logging, as the pool is already allocated */ @@ -1198,8 +1170,6 @@ if (target->type == LOG_TGT_TYPE_FILE) { rc = open(target->tgt_file.fname, O_WRONLY|O_APPEND|O_CREAT|O_NONBLOCK, 0660); if (rc < 0) { - LOGP(DLGLOBAL, LOGL_ERROR, "Cannot open log target '%s' as non-blocking I/O: %s\n", - name, strerror(errno)); talloc_free(wq); return -errno; } -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/26089 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Idb4215fa2f364e28c0bb73fb9975b6c9f50a46f6 Gerrit-Change-Number: 26089 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin at sysmocom.de> Gerrit-MessageType: newchange -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20211103/d87f42f9/attachment.htm>