pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-trx/+/42199?usp=email )
Change subject: fix(threads): centralize portable strerror handling ......................................................................
fix(threads): centralize portable strerror handling
- Move strerror_r portability logic to Utils - Add strerror_buf() helper - Simplify thread error logging in Threads.cpp
Change-Id: I642aff8a9f98823e117c4debd19384ddf5975039 --- M CommonLibs/Threads.cpp M CommonLibs/Utils.cpp M CommonLibs/Utils.h M CommonLibs/trx_rate_ctr.cpp M Transceiver52M/device/ipc/IPCDevice.cpp 5 files changed, 31 insertions(+), 9 deletions(-)
Approvals: laforge: Looks good to me, but someone else must approve pespin: Looks good to me, but someone else must approve fixeria: Looks good to me, approved Jenkins Builder: Verified
diff --git a/CommonLibs/Threads.cpp b/CommonLibs/Threads.cpp index 377a1b0..91e78d7 100644 --- a/CommonLibs/Threads.cpp +++ b/CommonLibs/Threads.cpp @@ -31,6 +31,7 @@ #include "Threads.h" #include "Timeval.h" #include "Logger.h" +#include "Utils.h"
extern "C" { #include <osmocom/core/thread.h> @@ -51,10 +52,10 @@ if (pthread_setname_np(selfid, name) == 0) { LOG(INFO) << "Thread "<< selfid << " (task " << tid << ") set name: " << name; } else { - char buf[256]; + char err_buf[256]; int err = errno; - char* err_str = strerror_r(err, buf, sizeof(buf)); - LOG(NOTICE) << "Thread "<< selfid << " (task " << tid << ") set name "" << name << "" failed: (" << err << ") " << err_str; + LOG(NOTICE) << "Thread "<< selfid << " (task " << tid << ") set name "" << name << "" failed: (" << err + << ") " << strerror_buf(err, err_buf, sizeof(err_buf)); } }
diff --git a/CommonLibs/Utils.cpp b/CommonLibs/Utils.cpp index 6703531..b07120d 100644 --- a/CommonLibs/Utils.cpp +++ b/CommonLibs/Utils.cpp @@ -17,6 +17,7 @@ #include <vector> #include <string> #include <sstream> +#include <cstring>
std::vectorstd::string comma_delimited_to_vector(const char* opt) { @@ -32,3 +33,19 @@ } return result; } + +char *strerror_buf(int err, char *buf, size_t buf_size) +{ + if (buf == nullptr || buf_size == 0) + return nullptr; + + char *err_str = buf; +#if defined(__GLIBC__) && defined(_GNU_SOURCE) + err_str = strerror_r(err, buf, buf_size); +#else + if (!strerror_r(err, buf, buf_size)) { + snprintf(buf, buf_size, "Unknown error %d", err); + } +#endif + return err_str; +} diff --git a/CommonLibs/Utils.h b/CommonLibs/Utils.h index 8e61a9f..b032eac 100644 --- a/CommonLibs/Utils.h +++ b/CommonLibs/Utils.h @@ -20,3 +20,5 @@ #include <string>
std::vectorstd::string comma_delimited_to_vector(const char* opt); + +char *strerror_buf(int err, char *buf, size_t buf_size); diff --git a/CommonLibs/trx_rate_ctr.cpp b/CommonLibs/trx_rate_ctr.cpp index 3806268..b722a84 100644 --- a/CommonLibs/trx_rate_ctr.cpp +++ b/CommonLibs/trx_rate_ctr.cpp @@ -66,6 +66,7 @@ } #include "Threads.h" #include "Logger.h" +#include "Utils.h"
/* Used in dev_ctrs_pending, when set it means that channel slot contains unused (non-pending) counter data */ @@ -224,7 +225,7 @@ dev_ctrs_pending[dev_ctr->chan] = *dev_ctr; if (osmo_timerfd_schedule(&dev_rate_ctr_timerfd, &next_sched, &intv_sched) < 0) { LOGC(DCTR, ERROR) << "Failed to schedule timerfd: " << errno - << " = "<< strerror_r(errno, err_buf, sizeof(err_buf)); + << " = "<< strerror_buf(errno, err_buf, sizeof(err_buf)); } dev_rate_ctr_mutex.unlock(); break; @@ -235,7 +236,7 @@ trx_ctrs_pending[trx_ctr->chan] = *trx_ctr; if (osmo_timerfd_schedule(&trx_rate_ctr_timerfd, &next_sched, &intv_sched) < 0) { LOGC(DCTR, ERROR) << "Failed to schedule timerfd: " << errno - << " = "<< strerror_r(errno, err_buf, sizeof(err_buf)); + << " = "<< strerror_buf(errno, err_buf, sizeof(err_buf)); } trx_rate_ctr_mutex.unlock(); break; diff --git a/Transceiver52M/device/ipc/IPCDevice.cpp b/Transceiver52M/device/ipc/IPCDevice.cpp index 1d2b89a..25eac44 100644 --- a/Transceiver52M/device/ipc/IPCDevice.cpp +++ b/Transceiver52M/device/ipc/IPCDevice.cpp @@ -31,6 +31,7 @@ #include "Threads.h" #include "IPCDevice.h" #include "smpl_buf.h" +#include "Utils.h"
extern "C" { #include <sys/mman.h> @@ -100,7 +101,7 @@ LOGP(DDEV, LOGL_NOTICE, "Opening shm path %s\n", shm_name); if ((fd = shm_open(shm_name, O_CREAT | O_RDWR, S_IRUSR | S_IWUSR)) < 0) { LOGP(DDEV, LOGL_ERROR, "shm_open %d: %s\n", errno, - strerror_r(errno, err_buf, sizeof(err_buf))); + strerror_buf(errno, err_buf, sizeof(err_buf))); rc = -errno; goto err_shm_open; } @@ -109,7 +110,7 @@ struct stat shm_stat; if (fstat(fd, &shm_stat) < 0) { LOGP(DDEV, LOGL_ERROR, "fstat %d: %s\n", errno, - strerror_r(errno, err_buf, sizeof(err_buf))); + strerror_buf(errno, err_buf, sizeof(err_buf))); rc = -errno; goto err_mmap; } @@ -119,7 +120,7 @@ LOGP(DDEV, LOGL_NOTICE, "mmaping shared memory fd %d (size=%zu)\n", fd, shm_len); if ((shm = mmap(NULL, shm_len, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0)) == MAP_FAILED) { LOGP(DDEV, LOGL_ERROR, "mmap %d: %s\n", errno, - strerror_r(errno, err_buf, sizeof(err_buf))); + strerror_buf(errno, err_buf, sizeof(err_buf))); rc = -errno; goto err_mmap; } @@ -850,7 +851,7 @@
if (select(max_fd + 1, &crfds, &cwfds, 0, &wait) < 0) { LOGP(DDEV, LOGL_ERROR, "select() failed: %s\n", - strerror_r(errno, err_buf, sizeof(err_buf))); + strerror_buf(errno, err_buf, sizeof(err_buf))); return; }
-
pespin