From b142600a7a107fb1e8793cadbde6acaf24b94a06 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 6 Feb 2022 11:25:41 +0100 Subject: [PATCH 01/41] Use flock() instead of UUCP-style locking for serial devices This replaces AcquireUUCPLockAndOpen() with AcquireLockAndOpen(). Depending on a compile-time option (--enable-uucp-locking), either flock() or UUCP-style lockfiles will be used as the underlying lock mechanism. The default is flock(). The plugins using these routines have been updated. --- common/io/Serial.cpp | 49 +++++++++++++++++++++++ configure.ac | 17 ++++++++ include/ola/io/Serial.h | 10 ++++- plugins/stageprofi/StageProfiDetector.cpp | 6 +-- plugins/usbpro/BaseUsbProWidget.cpp | 3 +- 5 files changed, 78 insertions(+), 7 deletions(-) diff --git a/common/io/Serial.cpp b/common/io/Serial.cpp index 9ffe6ca21b..d73543cdb8 100644 --- a/common/io/Serial.cpp +++ b/common/io/Serial.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #ifdef _WIN32 #define VC_EXTRALEAN #define WIN32_LEAN_AND_MEAN @@ -57,6 +58,7 @@ using std::string; namespace { +#ifdef UUCP_LOCKING string GetLockFile(const string &path) { const string base_name = ola::file::FilenameFromPath(path); return ola::file::JoinPaths(UUCP_LOCK_DIR, "LCK.." + base_name); @@ -113,6 +115,7 @@ bool RemoveLockFile(const string &lock_file) { } return true; } +#endif // UUCP_LOCKING } // namespace @@ -140,7 +143,41 @@ bool UIntToSpeedT(uint32_t value, speed_t *output) { return false; } +#ifdef HAVE_FLOCK +bool OpenAndFlock(const std::string &path, int oflag, int *fd) { + // First, check if the path exists, there's no point trying to open it if not + if (!FileExists(path)) { + OLA_INFO << "Device " << path << " doesn't exist."; + return false; + } + + // Now try to open the serial device. + if (!TryOpen(path, oflag, fd)) { + OLA_DEBUG << "Failed to open device " << path; + return false; + } + + if (flock(*fd, LOCK_EX | LOCK_NB) == -1) { + OLA_INFO << "Failed to flock() device " << path; + close(*fd); + return false; + } + +#if HAVE_SYS_IOCTL_H + // As a final safety mechanism, use ioctl(TIOCEXCL) if available to prevent + // further opens. + if (ioctl(*fd, TIOCEXCL) == -1) { + OLA_WARN << "TIOCEXCL " << path << " failed: " << strerror(errno); + close(*fd); + return false; + } +#endif // HAVE_SYS_IOCTL_H + return true; +} +#endif // HAVE_FLOCK + +#ifdef UUCP_LOCKING bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) { // This is rather tricky since there is no real convention for LCK files. // If it was only a single process doing the locking we could use fnctl as @@ -231,9 +268,11 @@ bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) { #endif // HAVE_SYS_IOCTL_H return true; } +#endif // UUCP_LOCKING void ReleaseUUCPLock(const std::string &path) { +#ifdef UUCP_LOCKING const string lock_file = GetLockFile(path); pid_t locked_pid; @@ -247,6 +286,16 @@ void ReleaseUUCPLock(const std::string &path) { OLA_INFO << "Released " << lock_file; } } +#endif /* else no action needed */ } + +bool AcquireLockAndOpen(const std::string &path, int oflag, int *fd) { +#ifdef UUCP_LOCKING + return AcquireUUCPLockAndOpen(path, oflag, fd); +#else + return OpenAndFlock(path, oflag, fd); +#endif +} + } // namespace io } // namespace ola diff --git a/configure.ac b/configure.ac index 59c1a1770a..790671796a 100644 --- a/configure.ac +++ b/configure.ac @@ -787,6 +787,22 @@ AC_ARG_ENABLE( [AS_HELP_STRING([--disable-doxygen-version], [Substitute the Doxygen version with latest, for the website])]) +# Use UUCP locking? +AC_ARG_ENABLE( + [uucp-locking], + [AS_HELP_STRING([--enable-uucp-locking], + [Use UUCP locking instead of flock()])]) + +AC_CHECK_FUNC([flock], [AC_DEFINE([HAVE_FLOCK], [1], + [Define if flock() is available]) + have_flock=yes], []) + +AS_IF([test "x$enable_uucp_locking" = xyes || test "x$have_flock" != xyes], + [AC_DEFINE([UUCP_LOCKING], [1], + [Define if UUCP locking should be used instead of flock()]) + SERIAL_LOCK_METHOD=UUCP], + [SERIAL_LOCK_METHOD=flock]) + # UUCP Lock directory AC_ARG_WITH([uucp-lock], [AS_HELP_STRING([--with-uucp-lock], @@ -1022,6 +1038,7 @@ Enable HTTP Server: ${have_microhttpd} RDM Responder Tests: ${enable_rdm_tests} Ja Rule: ${BUILDING_JA_RULE} Enabled Plugins:${PLUGINS} +Serial port locking: $SERIAL_LOCK_METHOD UUCP Lock Directory: $UUCPLOCK Now type 'make @<:@@:>@' diff --git a/include/ola/io/Serial.h b/include/ola/io/Serial.h index 17bbe15cd2..7b5c8f04ec 100644 --- a/include/ola/io/Serial.h +++ b/include/ola/io/Serial.h @@ -60,21 +60,27 @@ typedef enum { bool UIntToSpeedT(uint32_t value, speed_t *output); /** - * @brief Try to open the path, respecting UUCP locking. + * @brief Try to open the path and obtain a lock to control access * @param path the path to open * @param oflag flags passed to open * @param[out] fd a pointer to the fd which is returned. * @returns true if the open succeeded, false otherwise. * + * Depending on the compile-time configuration, this will use either flock() + * (the default) or UUCP locking. See: ./configure --enable-uucp-locking. + * * This fails-fast, it we can't get the lock immediately, we'll return false. */ -bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd); +bool AcquireLockAndOpen(const std::string &path, int oflag, int *fd); /** * @brief Remove a UUCP lock file for the device. * @param path The path to unlock. * * The lock is only removed if the PID matches. + * + * Does nothing if UUCP locking is not in use + * (see ./configure --enable-uucp-locking). */ void ReleaseUUCPLock(const std::string &path); } // namespace io diff --git a/plugins/stageprofi/StageProfiDetector.cpp b/plugins/stageprofi/StageProfiDetector.cpp index fab2908df9..7e54f97ad8 100644 --- a/plugins/stageprofi/StageProfiDetector.cpp +++ b/plugins/stageprofi/StageProfiDetector.cpp @@ -164,9 +164,9 @@ ConnectedDescriptor* StageProfiDetector::ConnectToUSB( struct termios newtio; int fd; - if (!ola::io::AcquireUUCPLockAndOpen(widget_path, - O_RDWR | O_NONBLOCK | O_NOCTTY, - &fd)) { + if (!ola::io::AcquireLockAndOpen(widget_path, + O_RDWR | O_NONBLOCK | O_NOCTTY, + &fd)) { return NULL; } diff --git a/plugins/usbpro/BaseUsbProWidget.cpp b/plugins/usbpro/BaseUsbProWidget.cpp index f92d2396eb..3589f68a4c 100644 --- a/plugins/usbpro/BaseUsbProWidget.cpp +++ b/plugins/usbpro/BaseUsbProWidget.cpp @@ -129,8 +129,7 @@ ola::io::ConnectedDescriptor *BaseUsbProWidget::OpenDevice( const string &path) { struct termios newtio; int fd; - if (!ola::io::AcquireUUCPLockAndOpen(path, O_RDWR | O_NONBLOCK | O_NOCTTY, - &fd)) { + if (!ola::io::AcquireLockAndOpen(path, O_RDWR | O_NONBLOCK | O_NOCTTY, &fd)) { return NULL; } From 05564726d6aebed391463206ebd0100533421fc7 Mon Sep 17 00:00:00 2001 From: Peter Newman Date: Mon, 7 Feb 2022 13:39:21 +0000 Subject: [PATCH 02/41] Ignore a codespell false positive due to line wrapping --- .codespellignorelines | 1 + 1 file changed, 1 insertion(+) diff --git a/.codespellignorelines b/.codespellignorelines index 7e38e74409..da03b13dca 100644 --- a/.codespellignorelines +++ b/.codespellignorelines @@ -177,3 +177,4 @@ import java.nio.ByteOrder; byte[] header = ByteBuffer.allocate(4).order(ByteOrder.nativeOrder()).putInt(headerContent).array(); int headerValue = ByteBuffer.wrap(header).order(ByteOrder.nativeOrder()).getInt(); # This is a very bodgy workaround to the fact that the pip install of the archive doesn't seem to work properly now on Travis + "uest\032\036.ola.rpc.STREAMING_NO_RESPONSEB\006\200\001" From dd34c1d61c973d74b7e912e14ba753f68ca7f4f0 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 13 Feb 2022 16:23:40 +0100 Subject: [PATCH 03/41] Always compile UUCP code There's not much to be gained from not compiling it. --- common/io/Serial.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/common/io/Serial.cpp b/common/io/Serial.cpp index d73543cdb8..749219d47b 100644 --- a/common/io/Serial.cpp +++ b/common/io/Serial.cpp @@ -58,7 +58,6 @@ using std::string; namespace { -#ifdef UUCP_LOCKING string GetLockFile(const string &path) { const string base_name = ola::file::FilenameFromPath(path); return ola::file::JoinPaths(UUCP_LOCK_DIR, "LCK.." + base_name); @@ -115,7 +114,6 @@ bool RemoveLockFile(const string &lock_file) { } return true; } -#endif // UUCP_LOCKING } // namespace @@ -177,7 +175,6 @@ bool OpenAndFlock(const std::string &path, int oflag, int *fd) { #endif // HAVE_FLOCK -#ifdef UUCP_LOCKING bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) { // This is rather tricky since there is no real convention for LCK files. // If it was only a single process doing the locking we could use fnctl as @@ -268,7 +265,6 @@ bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) { #endif // HAVE_SYS_IOCTL_H return true; } -#endif // UUCP_LOCKING void ReleaseUUCPLock(const std::string &path) { From cf4be9039c9346210ceeb82745dd8eb637dfcbec Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 13 Feb 2022 16:31:08 +0100 Subject: [PATCH 04/41] Rename UUCP lock routines RemoveLockFile -> RemoveUUCPLockFile GetLockFile -> GetUUCPLockFile --- common/io/Serial.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/common/io/Serial.cpp b/common/io/Serial.cpp index 749219d47b..206e4e191a 100644 --- a/common/io/Serial.cpp +++ b/common/io/Serial.cpp @@ -58,7 +58,7 @@ using std::string; namespace { -string GetLockFile(const string &path) { +string GetUUCPLockFile(const string &path) { const string base_name = ola::file::FilenameFromPath(path); return ola::file::JoinPaths(UUCP_LOCK_DIR, "LCK.." + base_name); } @@ -107,7 +107,7 @@ bool ProcessExists(pid_t pid) { #endif // _WIN32 } -bool RemoveLockFile(const string &lock_file) { +bool RemoveUUCPLockFile(const string &lock_file) { if (unlink(lock_file.c_str())) { OLA_WARN << "Failed to remove UUCP lock file: " << lock_file; return false; @@ -188,7 +188,7 @@ bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) { } // Second, clean up a stale lockfile. - const string lock_file = GetLockFile(path); + const string lock_file = GetUUCPLockFile(path); OLA_DEBUG << "Checking for " << lock_file; pid_t locked_pid; if (!GetPidFromFile(lock_file, &locked_pid)) { @@ -207,7 +207,7 @@ bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) { } // There is a race between the read & the unlink here. I'm not convinced it // can be solved. - if (!RemoveLockFile(lock_file)) { + if (!RemoveUUCPLockFile(lock_file)) { OLA_INFO << "Device " << path << " was locked by PID " << locked_pid << " which is no longer active, however failed to remove stale " << "lock file"; @@ -241,7 +241,7 @@ bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) { close(lock_fd); if (r != pid_file_contents.size()) { OLA_WARN << "Failed to write complete LCK file: " << lock_file; - RemoveLockFile(lock_file); + RemoveUUCPLockFile(lock_file); return false; } @@ -249,7 +249,7 @@ bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) { if (!TryOpen(path, oflag, fd)) { OLA_DEBUG << "Failed to open device " << path << " despite having the " << "lock file"; - RemoveLockFile(lock_file); + RemoveUUCPLockFile(lock_file); return false; } @@ -259,7 +259,7 @@ bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) { if (ioctl(*fd, TIOCEXCL) == -1) { OLA_WARN << "TIOCEXCL " << path << " failed: " << strerror(errno); close(*fd); - RemoveLockFile(lock_file); + RemoveUUCPLockFile(lock_file); return false; } #endif // HAVE_SYS_IOCTL_H @@ -269,7 +269,7 @@ bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) { void ReleaseUUCPLock(const std::string &path) { #ifdef UUCP_LOCKING - const string lock_file = GetLockFile(path); + const string lock_file = GetUUCPLockFile(path); pid_t locked_pid; if (!GetPidFromFile(lock_file, &locked_pid)) { @@ -278,7 +278,7 @@ void ReleaseUUCPLock(const std::string &path) { pid_t our_pid = getpid(); if (our_pid == locked_pid) { - if (RemoveLockFile(lock_file)) { + if (RemoveUUCPLockFile(lock_file)) { OLA_INFO << "Released " << lock_file; } } From 638daf7788a5e1c02fb1ca7eadd3861443fd152c Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 13 Feb 2022 16:33:10 +0100 Subject: [PATCH 05/41] Rename AcquireLockAndOpen -> AcquireLockAndOpenSerialPort --- common/io/Serial.cpp | 2 +- include/ola/io/Serial.h | 2 +- plugins/stageprofi/StageProfiDetector.cpp | 6 +++--- plugins/usbpro/BaseUsbProWidget.cpp | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/common/io/Serial.cpp b/common/io/Serial.cpp index 206e4e191a..233136d429 100644 --- a/common/io/Serial.cpp +++ b/common/io/Serial.cpp @@ -285,7 +285,7 @@ void ReleaseUUCPLock(const std::string &path) { #endif /* else no action needed */ } -bool AcquireLockAndOpen(const std::string &path, int oflag, int *fd) { +bool AcquireLockAndOpenSerialPort(const std::string &path, int oflag, int *fd) { #ifdef UUCP_LOCKING return AcquireUUCPLockAndOpen(path, oflag, fd); #else diff --git a/include/ola/io/Serial.h b/include/ola/io/Serial.h index 7b5c8f04ec..2e8291b2a9 100644 --- a/include/ola/io/Serial.h +++ b/include/ola/io/Serial.h @@ -71,7 +71,7 @@ bool UIntToSpeedT(uint32_t value, speed_t *output); * * This fails-fast, it we can't get the lock immediately, we'll return false. */ -bool AcquireLockAndOpen(const std::string &path, int oflag, int *fd); +bool AcquireLockAndOpenSerialPort(const std::string &path, int oflag, int *fd); /** * @brief Remove a UUCP lock file for the device. diff --git a/plugins/stageprofi/StageProfiDetector.cpp b/plugins/stageprofi/StageProfiDetector.cpp index 7e54f97ad8..3e16d84395 100644 --- a/plugins/stageprofi/StageProfiDetector.cpp +++ b/plugins/stageprofi/StageProfiDetector.cpp @@ -164,9 +164,9 @@ ConnectedDescriptor* StageProfiDetector::ConnectToUSB( struct termios newtio; int fd; - if (!ola::io::AcquireLockAndOpen(widget_path, - O_RDWR | O_NONBLOCK | O_NOCTTY, - &fd)) { + if (!ola::io::AcquireLockAndOpenSerialPort(widget_path, + O_RDWR | O_NONBLOCK | O_NOCTTY, + &fd)) { return NULL; } diff --git a/plugins/usbpro/BaseUsbProWidget.cpp b/plugins/usbpro/BaseUsbProWidget.cpp index 3589f68a4c..7edb6d5b62 100644 --- a/plugins/usbpro/BaseUsbProWidget.cpp +++ b/plugins/usbpro/BaseUsbProWidget.cpp @@ -129,7 +129,7 @@ ola::io::ConnectedDescriptor *BaseUsbProWidget::OpenDevice( const string &path) { struct termios newtio; int fd; - if (!ola::io::AcquireLockAndOpen(path, O_RDWR | O_NONBLOCK | O_NOCTTY, &fd)) { + if (!ola::io::AcquireLockAndOpenSerialPort(path, O_RDWR | O_NONBLOCK | O_NOCTTY, &fd)) { return NULL; } From 5d0cb396cce8ba75943e8b2adb235159bd81b0b2 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 13 Feb 2022 16:34:49 +0100 Subject: [PATCH 06/41] Add ReleaseSerialPortLock (to match AcquireLockAndOpenSerialPort) --- common/io/Serial.cpp | 9 +++++++-- include/ola/io/Serial.h | 11 ++++++----- plugins/stageprofi/StageProfiDetector.cpp | 2 +- plugins/usbpro/WidgetDetectorThread.cpp | 2 +- 4 files changed, 15 insertions(+), 9 deletions(-) diff --git a/common/io/Serial.cpp b/common/io/Serial.cpp index 233136d429..0a776e371d 100644 --- a/common/io/Serial.cpp +++ b/common/io/Serial.cpp @@ -268,7 +268,6 @@ bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) { void ReleaseUUCPLock(const std::string &path) { -#ifdef UUCP_LOCKING const string lock_file = GetUUCPLockFile(path); pid_t locked_pid; @@ -282,7 +281,6 @@ void ReleaseUUCPLock(const std::string &path) { OLA_INFO << "Released " << lock_file; } } -#endif /* else no action needed */ } bool AcquireLockAndOpenSerialPort(const std::string &path, int oflag, int *fd) { @@ -293,5 +291,12 @@ bool AcquireLockAndOpenSerialPort(const std::string &path, int oflag, int *fd) { #endif } +void ReleaseSerialPortLock(const std::string &path) { +#ifdef UUCP_LOCKING + ReleaseUUCPLock(path); +#endif // UUCP_LOCKING +} + + } // namespace io } // namespace ola diff --git a/include/ola/io/Serial.h b/include/ola/io/Serial.h index 2e8291b2a9..33944dbff3 100644 --- a/include/ola/io/Serial.h +++ b/include/ola/io/Serial.h @@ -74,15 +74,16 @@ bool UIntToSpeedT(uint32_t value, speed_t *output); bool AcquireLockAndOpenSerialPort(const std::string &path, int oflag, int *fd); /** - * @brief Remove a UUCP lock file for the device. + * @brief Release a lock for the serial port * @param path The path to unlock. * - * The lock is only removed if the PID matches. + * If UUCP locking was used (see AcquireLockAndOpenSerialPort), the lockfile + * will be removed (but only if the PID matches). * - * Does nothing if UUCP locking is not in use - * (see ./configure --enable-uucp-locking). + * Does nothing if flock() was used (see ./configure --enable-uucp-locking). */ -void ReleaseUUCPLock(const std::string &path); +void ReleaseSerialPortLock(const std::string &path); + } // namespace io } // namespace ola #endif // INCLUDE_OLA_IO_SERIAL_H_ diff --git a/plugins/stageprofi/StageProfiDetector.cpp b/plugins/stageprofi/StageProfiDetector.cpp index 3e16d84395..3f0a035387 100644 --- a/plugins/stageprofi/StageProfiDetector.cpp +++ b/plugins/stageprofi/StageProfiDetector.cpp @@ -126,7 +126,7 @@ void StageProfiDetector::ReleaseWidget(const std::string &widget_path) { // the map. DescriptorMap::iterator iter = m_usb_widgets.find(widget_path); if (iter != m_usb_widgets.end()) { - ola::io::ReleaseUUCPLock(widget_path); + ola::io::ReleaseSerialPortLock(widget_path); iter->second = NULL; return; } diff --git a/plugins/usbpro/WidgetDetectorThread.cpp b/plugins/usbpro/WidgetDetectorThread.cpp index 5f70ff52f0..692a3ee304 100644 --- a/plugins/usbpro/WidgetDetectorThread.cpp +++ b/plugins/usbpro/WidgetDetectorThread.cpp @@ -420,7 +420,7 @@ void WidgetDetectorThread::FreeDescriptor(ConnectedDescriptor *descriptor) { DescriptorInfo &descriptor_info = m_active_descriptors[descriptor]; m_active_paths.erase(descriptor_info.first); - io::ReleaseUUCPLock(descriptor_info.first); + io::ReleaseSerialPortLock(descriptor_info.first); m_active_descriptors.erase(descriptor); delete descriptor; } From 15a299b2cc9c199df95c48811f0f71489cb7368c Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 13 Feb 2022 16:35:10 +0100 Subject: [PATCH 07/41] Fix indentation --- common/io/Serial.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/io/Serial.cpp b/common/io/Serial.cpp index 0a776e371d..09357243ae 100644 --- a/common/io/Serial.cpp +++ b/common/io/Serial.cpp @@ -285,9 +285,9 @@ void ReleaseUUCPLock(const std::string &path) { bool AcquireLockAndOpenSerialPort(const std::string &path, int oflag, int *fd) { #ifdef UUCP_LOCKING - return AcquireUUCPLockAndOpen(path, oflag, fd); + return AcquireUUCPLockAndOpen(path, oflag, fd); #else - return OpenAndFlock(path, oflag, fd); + return OpenAndFlock(path, oflag, fd); #endif } From 10e4ac66a252685544f3d2a84e76d91323b4afc0 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 13 Feb 2022 18:06:29 +0100 Subject: [PATCH 08/41] OpenAndFlock: Return false if flock() is not available --- common/io/Serial.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/common/io/Serial.cpp b/common/io/Serial.cpp index 09357243ae..b48278f136 100644 --- a/common/io/Serial.cpp +++ b/common/io/Serial.cpp @@ -141,7 +141,6 @@ bool UIntToSpeedT(uint32_t value, speed_t *output) { return false; } -#ifdef HAVE_FLOCK bool OpenAndFlock(const std::string &path, int oflag, int *fd) { // First, check if the path exists, there's no point trying to open it if not if (!FileExists(path)) { @@ -155,11 +154,15 @@ bool OpenAndFlock(const std::string &path, int oflag, int *fd) { return false; } +#ifdef HAVE_FLOCK if (flock(*fd, LOCK_EX | LOCK_NB) == -1) { OLA_INFO << "Failed to flock() device " << path; close(*fd); return false; } +#else // HAVE_FLOCK + return false; +#endif // HAVE_FLOCK #if HAVE_SYS_IOCTL_H // As a final safety mechanism, use ioctl(TIOCEXCL) if available to prevent @@ -172,7 +175,6 @@ bool OpenAndFlock(const std::string &path, int oflag, int *fd) { #endif // HAVE_SYS_IOCTL_H return true; } -#endif // HAVE_FLOCK bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) { From d19563964192de8c2f34d0bcc15ffc2652c57eff Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 13 Feb 2022 21:10:18 +0100 Subject: [PATCH 09/41] Add a message when locking with flock() --- common/io/Serial.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/common/io/Serial.cpp b/common/io/Serial.cpp index b48278f136..53584806de 100644 --- a/common/io/Serial.cpp +++ b/common/io/Serial.cpp @@ -173,6 +173,8 @@ bool OpenAndFlock(const std::string &path, int oflag, int *fd) { return false; } #endif // HAVE_SYS_IOCTL_H + + OLA_INFO << "Locked " << path << " using flock()"; return true; } From ce8f9b4c6285cdf95159feed441adb6d05af16bd Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 13 Feb 2022 23:21:12 +0100 Subject: [PATCH 10/41] Restore documentation of deprecated UUCP locking routines --- include/ola/io/Serial.h | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/include/ola/io/Serial.h b/include/ola/io/Serial.h index 33944dbff3..15a3b40c32 100644 --- a/include/ola/io/Serial.h +++ b/include/ola/io/Serial.h @@ -59,6 +59,31 @@ typedef enum { */ bool UIntToSpeedT(uint32_t value, speed_t *output); +/** + * @brief Try to open the path, respecting UUCP locking. + * @param path the path to open + * @param oflag flags passed to open + * @param[out] fd a pointer to the fd which is returned. + * @returns true if the open succeeded, false otherwise. + * + * This fails-fast, it we can't get the lock immediately, we'll return false. + * + * @deprecated Use AcquireLockAndOpenSerialPort() instead. + * @see ReleaseUUCPLock() + */ +bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd); + +/** + * @brief Remove a UUCP lock file for the device. + * @param path The path to unlock. + * + * The lock is only removed if the PID matches. + * + * @deprecated Use ReleaseSerialPortLock() instead. + * @see AcquireUUCPLockAndOpen() + */ +void ReleaseUUCPLock(const std::string &path); + /** * @brief Try to open the path and obtain a lock to control access * @param path the path to open @@ -70,6 +95,8 @@ bool UIntToSpeedT(uint32_t value, speed_t *output); * (the default) or UUCP locking. See: ./configure --enable-uucp-locking. * * This fails-fast, it we can't get the lock immediately, we'll return false. + * + * @see ReleaseSerialPortLock() */ bool AcquireLockAndOpenSerialPort(const std::string &path, int oflag, int *fd); @@ -81,6 +108,8 @@ bool AcquireLockAndOpenSerialPort(const std::string &path, int oflag, int *fd); * will be removed (but only if the PID matches). * * Does nothing if flock() was used (see ./configure --enable-uucp-locking). + * + * @see AcquireLockAndOpenSerialPort() */ void ReleaseSerialPortLock(const std::string &path); From d9f2721e74b56a5404f3c974b07d9764ea2839f6 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 13 Feb 2022 23:31:44 +0100 Subject: [PATCH 11/41] OpenAndFlock: close FD if flock() is not available --- common/io/Serial.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/common/io/Serial.cpp b/common/io/Serial.cpp index 53584806de..d717a3ebd8 100644 --- a/common/io/Serial.cpp +++ b/common/io/Serial.cpp @@ -161,6 +161,7 @@ bool OpenAndFlock(const std::string &path, int oflag, int *fd) { return false; } #else // HAVE_FLOCK + close(*fd); return false; #endif // HAVE_FLOCK From 4ea7de4b5fc711143ee3c64980eeee0a2d127830 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 13 Feb 2022 23:46:25 +0100 Subject: [PATCH 12/41] Add extra spaces before comments --- common/io/Serial.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/io/Serial.cpp b/common/io/Serial.cpp index d717a3ebd8..0e3affd26e 100644 --- a/common/io/Serial.cpp +++ b/common/io/Serial.cpp @@ -160,10 +160,10 @@ bool OpenAndFlock(const std::string &path, int oflag, int *fd) { close(*fd); return false; } -#else // HAVE_FLOCK +#else // HAVE_FLOCK close(*fd); return false; -#endif // HAVE_FLOCK +#endif // HAVE_FLOCK #if HAVE_SYS_IOCTL_H // As a final safety mechanism, use ioctl(TIOCEXCL) if available to prevent From 6951d7652cfc39eca8bcc2bbb1614d94a4b46899 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sat, 19 Feb 2022 10:01:41 +0000 Subject: [PATCH 13/41] Wrap a long line Co-authored-by: Peter Newman --- plugins/usbpro/BaseUsbProWidget.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/plugins/usbpro/BaseUsbProWidget.cpp b/plugins/usbpro/BaseUsbProWidget.cpp index 7edb6d5b62..f36f4ec9a4 100644 --- a/plugins/usbpro/BaseUsbProWidget.cpp +++ b/plugins/usbpro/BaseUsbProWidget.cpp @@ -129,7 +129,9 @@ ola::io::ConnectedDescriptor *BaseUsbProWidget::OpenDevice( const string &path) { struct termios newtio; int fd; - if (!ola::io::AcquireLockAndOpenSerialPort(path, O_RDWR | O_NONBLOCK | O_NOCTTY, &fd)) { + if (!ola::io::AcquireLockAndOpenSerialPort(path, + O_RDWR | O_NONBLOCK | O_NOCTTY, + &fd)) { return NULL; } From 61752ab82a4dd8f813db9b42549ad9f8e3d49ebe Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sat, 19 Feb 2022 11:08:50 +0100 Subject: [PATCH 14/41] Add some more debug messages --- common/io/Serial.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/common/io/Serial.cpp b/common/io/Serial.cpp index 0e3affd26e..3ff5cb1ab8 100644 --- a/common/io/Serial.cpp +++ b/common/io/Serial.cpp @@ -161,6 +161,7 @@ bool OpenAndFlock(const std::string &path, int oflag, int *fd) { return false; } #else // HAVE_FLOCK + OLA_WARN << "Tried to flock " << path << ", but flock() is unavailable"; close(*fd); return false; #endif // HAVE_FLOCK @@ -299,6 +300,8 @@ bool AcquireLockAndOpenSerialPort(const std::string &path, int oflag, int *fd) { void ReleaseSerialPortLock(const std::string &path) { #ifdef UUCP_LOCKING ReleaseUUCPLock(path); +#else // UUCP_LOCKING + OLA_INFO << "No unlock necessary for " << path; #endif // UUCP_LOCKING } From d45704f4a65552ad12c7ada2057ef142012618ba Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sat, 19 Feb 2022 11:09:04 +0100 Subject: [PATCH 15/41] Add deprecation dates --- include/ola/io/Serial.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/ola/io/Serial.h b/include/ola/io/Serial.h index 15a3b40c32..04edebb971 100644 --- a/include/ola/io/Serial.h +++ b/include/ola/io/Serial.h @@ -68,7 +68,7 @@ bool UIntToSpeedT(uint32_t value, speed_t *output); * * This fails-fast, it we can't get the lock immediately, we'll return false. * - * @deprecated Use AcquireLockAndOpenSerialPort() instead. + * @deprecated Use AcquireLockAndOpenSerialPort() instead (19 Feb 2022). * @see ReleaseUUCPLock() */ bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd); @@ -79,7 +79,7 @@ bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd); * * The lock is only removed if the PID matches. * - * @deprecated Use ReleaseSerialPortLock() instead. + * @deprecated Use ReleaseSerialPortLock() instead (19 Feb 2022). * @see AcquireUUCPLockAndOpen() */ void ReleaseUUCPLock(const std::string &path); From 53b8c2cbd50ff2d895c5b294528ddcb81adaf05e Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sat, 19 Feb 2022 11:09:18 +0100 Subject: [PATCH 16/41] Add parentheses to make Doxygen link a reference --- include/ola/io/Serial.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/ola/io/Serial.h b/include/ola/io/Serial.h index 04edebb971..6ebc992898 100644 --- a/include/ola/io/Serial.h +++ b/include/ola/io/Serial.h @@ -104,7 +104,7 @@ bool AcquireLockAndOpenSerialPort(const std::string &path, int oflag, int *fd); * @brief Release a lock for the serial port * @param path The path to unlock. * - * If UUCP locking was used (see AcquireLockAndOpenSerialPort), the lockfile + * If UUCP locking was used (see AcquireLockAndOpenSerialPort()), the lockfile * will be removed (but only if the PID matches). * * Does nothing if flock() was used (see ./configure --enable-uucp-locking). From 9cd59a88b34451b165faa94aebe342c9737c0ff4 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sat, 19 Feb 2022 17:09:35 +0100 Subject: [PATCH 17/41] Add SerialLockTester --- common/io/Makefile.mk | 7 +++- common/io/SerialLockTester.cpp | 59 ++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 common/io/SerialLockTester.cpp diff --git a/common/io/Makefile.mk b/common/io/Makefile.mk index a19c5bafe2..fb64291fce 100644 --- a/common/io/Makefile.mk +++ b/common/io/Makefile.mk @@ -47,7 +47,8 @@ test_programs += \ common/io/MemoryBlockTester \ common/io/SelectServerTester \ common/io/StreamTester \ - common/io/TimeoutManagerTester + common/io/TimeoutManagerTester \ + common/io/SerialLockTester common_io_IOQueueTester_SOURCES = common/io/IOQueueTest.cpp common_io_IOQueueTester_CXXFLAGS = $(COMMON_TESTING_FLAGS) @@ -78,3 +79,7 @@ common_io_StreamTester_SOURCES = common/io/InputStreamTest.cpp \ common/io/OutputStreamTest.cpp common_io_StreamTester_CXXFLAGS = $(COMMON_TESTING_FLAGS) common_io_StreamTester_LDADD = $(COMMON_TESTING_LIBS) + +common_io_SerialLockTester_SOURCES = common/io/SerialLockTester.cpp +common_io_SerialLockTester_CXXFLAGS = $(COMMON_TESTING_FLAGS) +common_io_SerialLockTester_LDADD = $(COMMON_TESTING_LIBS) diff --git a/common/io/SerialLockTester.cpp b/common/io/SerialLockTester.cpp new file mode 100644 index 0000000000..4a31b04979 --- /dev/null +++ b/common/io/SerialLockTester.cpp @@ -0,0 +1,59 @@ +/* + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * + * SerialLockTester.cpp + * Test for serial port locking + * Copyright (C) 2022 Thomas White + */ + +#if HAVE_CONFIG_H +#include +#endif // HAVE_CONFIG_H + +#include +#include +#include + +#include "ola/io/Serial.h" +#include "ola/testing/TestUtils.h" + +class SerialLockTest: public CppUnit::TestFixture { + public: + CPPUNIT_TEST_SUITE(SerialLockTest); + CPPUNIT_TEST(testLock); + CPPUNIT_TEST_SUITE_END(); + + public: + void testLock(); +}; + + +CPPUNIT_TEST_SUITE_REGISTRATION(SerialLockTest); + +void SerialLockTest::testLock() { + + bool r1, r2; + int fd; + const std::string path = "ChangeLog"; + + r1 = ola::io::AcquireLockAndOpenSerialPort(path, O_RDWR, &fd); + OLA_ASSERT_TRUE(r1); + + r2 = ola::io::AcquireLockAndOpenSerialPort(path, O_RDWR, &fd); + OLA_ASSERT_FALSE(r2); + + ola::io::ReleaseSerialPortLock(path); + close(fd); +}; From 3e072f17556e32101e3b617f3102ade67ce736fb Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sat, 19 Feb 2022 17:09:51 +0100 Subject: [PATCH 18/41] Don't fail if TIOCEXCL doesn't work --- common/io/Serial.cpp | 5 ----- common/io/SerialLockTester.cpp | 8 ++++---- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/common/io/Serial.cpp b/common/io/Serial.cpp index 3ff5cb1ab8..505653c9af 100644 --- a/common/io/Serial.cpp +++ b/common/io/Serial.cpp @@ -171,8 +171,6 @@ bool OpenAndFlock(const std::string &path, int oflag, int *fd) { // further opens. if (ioctl(*fd, TIOCEXCL) == -1) { OLA_WARN << "TIOCEXCL " << path << " failed: " << strerror(errno); - close(*fd); - return false; } #endif // HAVE_SYS_IOCTL_H @@ -264,9 +262,6 @@ bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) { // further opens. if (ioctl(*fd, TIOCEXCL) == -1) { OLA_WARN << "TIOCEXCL " << path << " failed: " << strerror(errno); - close(*fd); - RemoveUUCPLockFile(lock_file); - return false; } #endif // HAVE_SYS_IOCTL_H return true; diff --git a/common/io/SerialLockTester.cpp b/common/io/SerialLockTester.cpp index 4a31b04979..eb06abf9ed 100644 --- a/common/io/SerialLockTester.cpp +++ b/common/io/SerialLockTester.cpp @@ -45,15 +45,15 @@ CPPUNIT_TEST_SUITE_REGISTRATION(SerialLockTest); void SerialLockTest::testLock() { bool r1, r2; - int fd; + int fd1, fd2; const std::string path = "ChangeLog"; - r1 = ola::io::AcquireLockAndOpenSerialPort(path, O_RDWR, &fd); + r1 = ola::io::AcquireLockAndOpenSerialPort(path, O_RDWR, &fd1); OLA_ASSERT_TRUE(r1); - r2 = ola::io::AcquireLockAndOpenSerialPort(path, O_RDWR, &fd); + r2 = ola::io::AcquireLockAndOpenSerialPort(path, O_RDWR, &fd2); OLA_ASSERT_FALSE(r2); ola::io::ReleaseSerialPortLock(path); - close(fd); + close(fd1); }; From 4bcf0f6f35fd447776edd868ea8b35d960441408 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 20 Feb 2022 21:44:13 +0100 Subject: [PATCH 19/41] SerialLockTester: formatting fixes --- common/io/SerialLockTester.cpp | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/common/io/SerialLockTester.cpp b/common/io/SerialLockTester.cpp index eb06abf9ed..b50febf3aa 100644 --- a/common/io/SerialLockTester.cpp +++ b/common/io/SerialLockTester.cpp @@ -22,6 +22,7 @@ #include #endif // HAVE_CONFIG_H +#include #include #include #include @@ -43,17 +44,16 @@ class SerialLockTest: public CppUnit::TestFixture { CPPUNIT_TEST_SUITE_REGISTRATION(SerialLockTest); void SerialLockTest::testLock() { + bool r1, r2; + int fd1, fd2; + const std::string path = "ChangeLog"; - bool r1, r2; - int fd1, fd2; - const std::string path = "ChangeLog"; + r1 = ola::io::AcquireLockAndOpenSerialPort(path, O_RDWR, &fd1); + OLA_ASSERT_TRUE(r1); - r1 = ola::io::AcquireLockAndOpenSerialPort(path, O_RDWR, &fd1); - OLA_ASSERT_TRUE(r1); + r2 = ola::io::AcquireLockAndOpenSerialPort(path, O_RDWR, &fd2); + OLA_ASSERT_FALSE(r2); - r2 = ola::io::AcquireLockAndOpenSerialPort(path, O_RDWR, &fd2); - OLA_ASSERT_FALSE(r2); - - ola::io::ReleaseSerialPortLock(path); - close(fd1); -}; + ola::io::ReleaseSerialPortLock(path); + close(fd1); +} From d035bde6fafa183a3310fbaaba2362223cf59a67 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 20 Feb 2022 22:02:17 +0100 Subject: [PATCH 20/41] SerialLockTester: rearrange includes --- common/io/SerialLockTester.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/io/SerialLockTester.cpp b/common/io/SerialLockTester.cpp index b50febf3aa..0eed31f3d7 100644 --- a/common/io/SerialLockTester.cpp +++ b/common/io/SerialLockTester.cpp @@ -22,9 +22,9 @@ #include #endif // HAVE_CONFIG_H -#include #include #include +#include #include #include "ola/io/Serial.h" From 379db11dbe251e73303e03539fababe0476505b3 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 20 Feb 2022 22:23:27 +0100 Subject: [PATCH 21/41] SerialLockTester: Rearrange includes again --- common/io/SerialLockTester.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/io/SerialLockTester.cpp b/common/io/SerialLockTester.cpp index 0eed31f3d7..099183e3db 100644 --- a/common/io/SerialLockTester.cpp +++ b/common/io/SerialLockTester.cpp @@ -24,8 +24,8 @@ #include #include -#include #include +#include #include "ola/io/Serial.h" #include "ola/testing/TestUtils.h" From 662b6e95032da50dea46281a2a07865571398f37 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Mon, 21 Feb 2022 09:09:29 +0100 Subject: [PATCH 22/41] SerialLockTester: Create the test file --- common/io/SerialLockTester.cpp | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/common/io/SerialLockTester.cpp b/common/io/SerialLockTester.cpp index 099183e3db..11474815e5 100644 --- a/common/io/SerialLockTester.cpp +++ b/common/io/SerialLockTester.cpp @@ -28,6 +28,7 @@ #include #include "ola/io/Serial.h" +#include "ola/io/IOUtils.h" #include "ola/testing/TestUtils.h" class SerialLockTest: public CppUnit::TestFixture { @@ -45,8 +46,19 @@ CPPUNIT_TEST_SUITE_REGISTRATION(SerialLockTest); void SerialLockTest::testLock() { bool r1, r2; - int fd1, fd2; - const std::string path = "ChangeLog"; + int fd1, fd2, fd3; + const std::string path = "serialLockTestFile"; + + OLA_ASSERT_FALSE(ola::io::FileExists(path)); + + fd3 = open(path.c_str(), O_CREAT | O_RDWR, + S_IRUSR | S_IWUSR +#ifndef _WIN32 + | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH +#endif // !_WIN32 + ); // NOLINT(whitespace/parens) + OLA_ASSERT_FALSE(fd3 < 0); + close(fd3); r1 = ola::io::AcquireLockAndOpenSerialPort(path, O_RDWR, &fd1); OLA_ASSERT_TRUE(r1); @@ -56,4 +68,6 @@ void SerialLockTest::testLock() { ola::io::ReleaseSerialPortLock(path); close(fd1); + + OLA_ASSERT_FALSE(unlink(path.c_str())); } From b394083e5644a1492af3743f3f1fd154f218af7b Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sat, 5 Mar 2022 10:29:01 +0000 Subject: [PATCH 23/41] Update include/ola/io/Serial.h Co-authored-by: Peter Newman --- include/ola/io/Serial.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/ola/io/Serial.h b/include/ola/io/Serial.h index 6ebc992898..2d72040cce 100644 --- a/include/ola/io/Serial.h +++ b/include/ola/io/Serial.h @@ -68,7 +68,7 @@ bool UIntToSpeedT(uint32_t value, speed_t *output); * * This fails-fast, it we can't get the lock immediately, we'll return false. * - * @deprecated Use AcquireLockAndOpenSerialPort() instead (19 Feb 2022). + * @deprecated Use the more general AcquireLockAndOpenSerialPort() instead (19 Feb 2022). * @see ReleaseUUCPLock() */ bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd); From 8ca5f345e45859f44abe1f3177d573ed26b88974 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sat, 5 Mar 2022 10:29:57 +0000 Subject: [PATCH 24/41] Remove pre-processor comment Co-authored-by: Peter Newman --- common/io/Serial.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/io/Serial.cpp b/common/io/Serial.cpp index 505653c9af..4f9807bc52 100644 --- a/common/io/Serial.cpp +++ b/common/io/Serial.cpp @@ -160,7 +160,7 @@ bool OpenAndFlock(const std::string &path, int oflag, int *fd) { close(*fd); return false; } -#else // HAVE_FLOCK +#else OLA_WARN << "Tried to flock " << path << ", but flock() is unavailable"; close(*fd); return false; From 11b2f40ae3a1a7b5be7bb741682fb66850e4f0e6 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sat, 5 Mar 2022 10:30:29 +0000 Subject: [PATCH 25/41] Mention flock() when saying "No unlock necessary" Co-authored-by: Peter Newman --- common/io/Serial.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/io/Serial.cpp b/common/io/Serial.cpp index 4f9807bc52..5fc68c38bc 100644 --- a/common/io/Serial.cpp +++ b/common/io/Serial.cpp @@ -296,7 +296,7 @@ void ReleaseSerialPortLock(const std::string &path) { #ifdef UUCP_LOCKING ReleaseUUCPLock(path); #else // UUCP_LOCKING - OLA_INFO << "No unlock necessary for " << path; + OLA_INFO << "No unlock necessary with for " << path << " as we're using flock()"; #endif // UUCP_LOCKING } From ff04282659ff5a0616cc111a1c5efe969648cf14 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sat, 5 Mar 2022 10:31:09 +0000 Subject: [PATCH 26/41] Update include/ola/io/Serial.h Co-authored-by: Peter Newman --- include/ola/io/Serial.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/ola/io/Serial.h b/include/ola/io/Serial.h index 2d72040cce..e5cd84b760 100644 --- a/include/ola/io/Serial.h +++ b/include/ola/io/Serial.h @@ -79,7 +79,7 @@ bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd); * * The lock is only removed if the PID matches. * - * @deprecated Use ReleaseSerialPortLock() instead (19 Feb 2022). + * @deprecated Use the more general ReleaseSerialPortLock() instead (19 Feb 2022). * @see AcquireUUCPLockAndOpen() */ void ReleaseUUCPLock(const std::string &path); From 613882317ec2d7a2dc7835a55e3f5973b5128310 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sat, 5 Mar 2022 12:52:47 +0000 Subject: [PATCH 27/41] Update common/io/SerialLockTester.cpp Co-authored-by: Peter Newman --- common/io/SerialLockTester.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/common/io/SerialLockTester.cpp b/common/io/SerialLockTester.cpp index 11474815e5..38b75a8ddd 100644 --- a/common/io/SerialLockTester.cpp +++ b/common/io/SerialLockTester.cpp @@ -63,8 +63,7 @@ void SerialLockTest::testLock() { r1 = ola::io::AcquireLockAndOpenSerialPort(path, O_RDWR, &fd1); OLA_ASSERT_TRUE(r1); - r2 = ola::io::AcquireLockAndOpenSerialPort(path, O_RDWR, &fd2); - OLA_ASSERT_FALSE(r2); + OLA_ASSERT_FALSE(ola::io::AcquireLockAndOpenSerialPort(path, O_RDWR, &fd2)); ola::io::ReleaseSerialPortLock(path); close(fd1); From 3d9ae1d9cbbde4c9cb9c46e8ac210d2b95a593b9 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 6 Mar 2022 11:48:05 +0100 Subject: [PATCH 28/41] Fix debug message and over-long line --- common/io/Serial.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/io/Serial.cpp b/common/io/Serial.cpp index 5fc68c38bc..8eb082848c 100644 --- a/common/io/Serial.cpp +++ b/common/io/Serial.cpp @@ -296,7 +296,7 @@ void ReleaseSerialPortLock(const std::string &path) { #ifdef UUCP_LOCKING ReleaseUUCPLock(path); #else // UUCP_LOCKING - OLA_INFO << "No unlock necessary with for " << path << " as we're using flock()"; + OLA_INFO << "No unlock necessary for " << path << " as we're using flock()"; #endif // UUCP_LOCKING } From 7fe0fa5323bae8a9135b9213564dd6a37966e422 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 6 Mar 2022 11:50:08 +0100 Subject: [PATCH 29/41] SerialLockTester: Eliminate unused variable --- common/io/SerialLockTester.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/io/SerialLockTester.cpp b/common/io/SerialLockTester.cpp index 38b75a8ddd..0ae8ad163c 100644 --- a/common/io/SerialLockTester.cpp +++ b/common/io/SerialLockTester.cpp @@ -45,7 +45,7 @@ class SerialLockTest: public CppUnit::TestFixture { CPPUNIT_TEST_SUITE_REGISTRATION(SerialLockTest); void SerialLockTest::testLock() { - bool r1, r2; + bool r1; int fd1, fd2, fd3; const std::string path = "serialLockTestFile"; From f9dec96215e6acd0ef4e4c8608fdc9e139165ed6 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 6 Mar 2022 18:50:36 +0100 Subject: [PATCH 30/41] Split serial port locking into three separate steps UUCP locking: performed if requested at configure time flock(): performed if the flock call is available (always?) ioctl(fd, TIOCEXCL): performed if sys/ioctl.h is available It's possible that all three will be done, but only one of them really has to work. If any of the selected methods fail, however, the port opening will be aborted. --- common/io/Serial.cpp | 140 +++++++++++++++++++-------------- common/io/SerialLockTester.cpp | 37 ++++++--- configure.ac | 9 ++- 3 files changed, 111 insertions(+), 75 deletions(-) diff --git a/common/io/Serial.cpp b/common/io/Serial.cpp index 8eb082848c..fece102383 100644 --- a/common/io/Serial.cpp +++ b/common/io/Serial.cpp @@ -141,57 +141,12 @@ bool UIntToSpeedT(uint32_t value, speed_t *output) { return false; } -bool OpenAndFlock(const std::string &path, int oflag, int *fd) { - // First, check if the path exists, there's no point trying to open it if not - if (!FileExists(path)) { - OLA_INFO << "Device " << path << " doesn't exist."; - return false; - } - - // Now try to open the serial device. - if (!TryOpen(path, oflag, fd)) { - OLA_DEBUG << "Failed to open device " << path; - return false; - } - -#ifdef HAVE_FLOCK - if (flock(*fd, LOCK_EX | LOCK_NB) == -1) { - OLA_INFO << "Failed to flock() device " << path; - close(*fd); - return false; - } -#else - OLA_WARN << "Tried to flock " << path << ", but flock() is unavailable"; - close(*fd); - return false; -#endif // HAVE_FLOCK - -#if HAVE_SYS_IOCTL_H - // As a final safety mechanism, use ioctl(TIOCEXCL) if available to prevent - // further opens. - if (ioctl(*fd, TIOCEXCL) == -1) { - OLA_WARN << "TIOCEXCL " << path << " failed: " << strerror(errno); - } -#endif // HAVE_SYS_IOCTL_H - - OLA_INFO << "Locked " << path << " using flock()"; - return true; -} - -bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) { +bool AcquireUUCPLock(const std::string &path) { // This is rather tricky since there is no real convention for LCK files. // If it was only a single process doing the locking we could use fnctl as // described in 55.6 of the Linux Programming Interface book. - // First, check if the path exists, there's no point trying to open it if not - if (!FileExists(path)) { - OLA_INFO << "Device " << path << " doesn't exist, so there's no point " - "trying to acquire a lock"; - return false; - } - - // Second, clean up a stale lockfile. const string lock_file = GetUUCPLockFile(path); OLA_DEBUG << "Checking for " << lock_file; pid_t locked_pid; @@ -200,6 +155,7 @@ bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) { return false; } + // Clean up a stale lockfile. if (locked_pid) { // This will return false even if we have the lock, this is what we want // since different plugins may try to open the same serial port - see issue @@ -249,21 +205,52 @@ bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) { return false; } + return true; +} + +bool LockTIOCEXCL(int fd, const std::string &path) { +#if HAVE_SYS_IOCTL_H + // This is a final safety mechanism, on top of UUCP locking or flock() + if (ioctl(fd, TIOCEXCL) == -1) { + OLA_WARN << "TIOCEXCL " << path << " failed: " << strerror(errno); + return false; + } else { + OLA_INFO << "Set TIOCEXCL on " << path; + } +#else + OLA_INFO << "Not setting " << path << " - ioctl.h unavailable"; +#endif // HAVE_SYS_IOCTL_H + return true; +} + +bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) { + + // First, check if the path exists, there's no point trying to open it if not + if (!FileExists(path)) { + OLA_INFO << "Device " << path << " doesn't exist, so there's no point " + "trying to acquire a lock"; + return false; + } + + if (!AcquireUUCPLock(path)) { + return false; + } + // Now try to open the serial device. if (!TryOpen(path, oflag, fd)) { OLA_DEBUG << "Failed to open device " << path << " despite having the " << "lock file"; - RemoveUUCPLockFile(lock_file); + ReleaseUUCPLock(path); + close(*fd); return false; } -#if HAVE_SYS_IOCTL_H - // As a final safety mechanism, use ioctl(TIOCEXCL) if available to prevent - // further opens. - if (ioctl(*fd, TIOCEXCL) == -1) { - OLA_WARN << "TIOCEXCL " << path << " failed: " << strerror(errno); + if (!LockTIOCEXCL(*fd, path)) { + ReleaseUUCPLock(path); + close(*fd); + return false; } -#endif // HAVE_SYS_IOCTL_H + return true; } @@ -285,19 +272,56 @@ void ReleaseUUCPLock(const std::string &path) { } bool AcquireLockAndOpenSerialPort(const std::string &path, int oflag, int *fd) { + + // First, check if the path exists, there's no point trying to open it if not + if (!FileExists(path)) { + OLA_INFO << "Device " << path << " doesn't exist, so there's no point " + "trying to acquire a lock"; + return false; + } + +#ifdef UUCP_LOCKING + if (!AcquireUUCPLock(path)) { + return false; + } +#endif + + // Now try to open the serial device. + if (!TryOpen(path, oflag, fd)) { #ifdef UUCP_LOCKING - return AcquireUUCPLockAndOpen(path, oflag, fd); + OLA_DEBUG << "Failed to open device " << path << " despite having the " + << "lock file"; + ReleaseUUCPLock(path); #else - return OpenAndFlock(path, oflag, fd); + OLA_DEBUG << "Failed to open device " << path; +#endif + return false; + } + +#ifdef HAVE_FLOCK + if (flock(*fd, LOCK_EX | LOCK_NB) == -1) { + OLA_INFO << "Failed to flock() device " << path; + close(*fd); + return false; + } else { + OLA_INFO << "Locked " << path << " using flock()"; + } #endif + + if (!LockTIOCEXCL(*fd, path)) { + close(*fd); + return false; + } + + return true; } void ReleaseSerialPortLock(const std::string &path) { #ifdef UUCP_LOCKING ReleaseUUCPLock(path); -#else // UUCP_LOCKING - OLA_INFO << "No unlock necessary for " << path << " as we're using flock()"; -#endif // UUCP_LOCKING +#else + OLA_INFO << "No unlock necessary for " << path << " (UUCP locking not in use)"; +#endif } diff --git a/common/io/SerialLockTester.cpp b/common/io/SerialLockTester.cpp index 0ae8ad163c..7c97c1541b 100644 --- a/common/io/SerialLockTester.cpp +++ b/common/io/SerialLockTester.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include "ola/io/Serial.h" #include "ola/io/IOUtils.h" @@ -45,28 +46,38 @@ class SerialLockTest: public CppUnit::TestFixture { CPPUNIT_TEST_SUITE_REGISTRATION(SerialLockTest); void SerialLockTest::testLock() { - bool r1; - int fd1, fd2, fd3; - const std::string path = "serialLockTestFile"; + int fd; + pid_t our_pid = getpid(); - OLA_ASSERT_FALSE(ola::io::FileExists(path)); + std::stringstream str; + str << "serialLockTestFile." << our_pid; + const std::string path = str.str(); - fd3 = open(path.c_str(), O_CREAT | O_RDWR, + OLA_ASSERT_FALSE_MSG(ola::io::FileExists(path), + "Test file already exists"); + + fd = open(path.c_str(), O_CREAT | O_RDWR, S_IRUSR | S_IWUSR #ifndef _WIN32 | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH #endif // !_WIN32 ); // NOLINT(whitespace/parens) - OLA_ASSERT_FALSE(fd3 < 0); - close(fd3); + OLA_ASSERT_FALSE_MSG(fd < 0, "Couldn't create test file"); + +#ifdef HAVE_FLOCK + OLA_ASSERT_TRUE(flock(fd, LOCK_EX | LOCK_NB) != -1); +#else - r1 = ola::io::AcquireLockAndOpenSerialPort(path, O_RDWR, &fd1); - OLA_ASSERT_TRUE(r1); +#ifdef UUCP_LOCKING + // flock() is not available, but UUCP locking was selected. OK. +#else + OLA_FAIL("Not using UUCP locking, and flock() is not available"); +#endif - OLA_ASSERT_FALSE(ola::io::AcquireLockAndOpenSerialPort(path, O_RDWR, &fd2)); +#endif - ola::io::ReleaseSerialPortLock(path); - close(fd1); + close(fd); - OLA_ASSERT_FALSE(unlink(path.c_str())); + OLA_ASSERT_FALSE_MSG(unlink(path.c_str()), + "Couldn't delete test file"); } diff --git a/configure.ac b/configure.ac index 790671796a..99570a15d2 100644 --- a/configure.ac +++ b/configure.ac @@ -797,11 +797,12 @@ AC_CHECK_FUNC([flock], [AC_DEFINE([HAVE_FLOCK], [1], [Define if flock() is available]) have_flock=yes], []) +UUCP_LOCKING="no" AS_IF([test "x$enable_uucp_locking" = xyes || test "x$have_flock" != xyes], [AC_DEFINE([UUCP_LOCKING], [1], - [Define if UUCP locking should be used instead of flock()]) - SERIAL_LOCK_METHOD=UUCP], - [SERIAL_LOCK_METHOD=flock]) + [Define if UUCP locking should be used]) + UUCP_LOCKING=yes], + []) # UUCP Lock directory AC_ARG_WITH([uucp-lock], @@ -1038,7 +1039,7 @@ Enable HTTP Server: ${have_microhttpd} RDM Responder Tests: ${enable_rdm_tests} Ja Rule: ${BUILDING_JA_RULE} Enabled Plugins:${PLUGINS} -Serial port locking: $SERIAL_LOCK_METHOD +UUCP Locking: $UUCP_LOCKING UUCP Lock Directory: $UUCPLOCK Now type 'make @<:@@:>@' From 0bfe20071286778ff6fac81377f7f8f57c491ee6 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 6 Mar 2022 22:20:41 +0100 Subject: [PATCH 31/41] Formatting --- common/io/Serial.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/common/io/Serial.cpp b/common/io/Serial.cpp index fece102383..7bd4a5520c 100644 --- a/common/io/Serial.cpp +++ b/common/io/Serial.cpp @@ -224,7 +224,6 @@ bool LockTIOCEXCL(int fd, const std::string &path) { } bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) { - // First, check if the path exists, there's no point trying to open it if not if (!FileExists(path)) { OLA_INFO << "Device " << path << " doesn't exist, so there's no point " @@ -272,7 +271,6 @@ void ReleaseUUCPLock(const std::string &path) { } bool AcquireLockAndOpenSerialPort(const std::string &path, int oflag, int *fd) { - // First, check if the path exists, there's no point trying to open it if not if (!FileExists(path)) { OLA_INFO << "Device " << path << " doesn't exist, so there's no point " @@ -320,7 +318,7 @@ void ReleaseSerialPortLock(const std::string &path) { #ifdef UUCP_LOCKING ReleaseUUCPLock(path); #else - OLA_INFO << "No unlock necessary for " << path << " (UUCP locking not in use)"; + OLA_INFO << "No unlock necessary for " << path << " (UUCP locking not used)"; #endif } From 9fe41fe4e30bbe58aa75419d68cf76e479d10811 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 6 Mar 2022 22:38:06 +0100 Subject: [PATCH 32/41] Fix include order --- common/io/SerialLockTester.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/io/SerialLockTester.cpp b/common/io/SerialLockTester.cpp index 7c97c1541b..69a8a83d5a 100644 --- a/common/io/SerialLockTester.cpp +++ b/common/io/SerialLockTester.cpp @@ -22,11 +22,11 @@ #include #endif // HAVE_CONFIG_H +#include #include #include #include #include -#include #include "ola/io/Serial.h" #include "ola/io/IOUtils.h" From f60bd33aa47aa790c760ed7b81dcdf1eb55bd348 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 20 Mar 2022 10:11:48 +0100 Subject: [PATCH 33/41] Clarify info message about TIOCEXCL Co-authored-by: Peter Newman --- common/io/Serial.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/io/Serial.cpp b/common/io/Serial.cpp index 7bd4a5520c..742bcdfa55 100644 --- a/common/io/Serial.cpp +++ b/common/io/Serial.cpp @@ -218,7 +218,7 @@ bool LockTIOCEXCL(int fd, const std::string &path) { OLA_INFO << "Set TIOCEXCL on " << path; } #else - OLA_INFO << "Not setting " << path << " - ioctl.h unavailable"; + OLA_INFO << "Not setting TIOCEXCL on " << path << " - ioctl.h unavailable"; #endif // HAVE_SYS_IOCTL_H return true; } From 4806d310612590da005e73f882d0f307ed71ffcd Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 20 Mar 2022 10:12:16 +0100 Subject: [PATCH 34/41] Restore deleted comment Co-authored-by: Peter Newman --- common/io/Serial.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/common/io/Serial.cpp b/common/io/Serial.cpp index 742bcdfa55..d5f3a9a47b 100644 --- a/common/io/Serial.cpp +++ b/common/io/Serial.cpp @@ -244,6 +244,8 @@ bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) { return false; } + // As a final safety mechanism, use ioctl(TIOCEXCL) if available to prevent + // further opens. if (!LockTIOCEXCL(*fd, path)) { ReleaseUUCPLock(path); close(*fd); From 0dfa3f961ac61981e0ff06628055a88c49bd3b16 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 20 Mar 2022 10:12:43 +0100 Subject: [PATCH 35/41] Add explanatory comment to #endif Co-authored-by: Peter Newman --- common/io/Serial.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/io/Serial.cpp b/common/io/Serial.cpp index d5f3a9a47b..3993a478f3 100644 --- a/common/io/Serial.cpp +++ b/common/io/Serial.cpp @@ -321,7 +321,7 @@ void ReleaseSerialPortLock(const std::string &path) { ReleaseUUCPLock(path); #else OLA_INFO << "No unlock necessary for " << path << " (UUCP locking not used)"; -#endif +#endif // UUCP_LOCKING } From f97bc8df2ba463661cd9736676bd2145455548c0 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 20 Mar 2022 10:13:16 +0100 Subject: [PATCH 36/41] SerialLockTester: Add ".pid" to test filename Co-authored-by: Peter Newman --- common/io/SerialLockTester.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/io/SerialLockTester.cpp b/common/io/SerialLockTester.cpp index 69a8a83d5a..d20d8f5daf 100644 --- a/common/io/SerialLockTester.cpp +++ b/common/io/SerialLockTester.cpp @@ -50,7 +50,7 @@ void SerialLockTest::testLock() { pid_t our_pid = getpid(); std::stringstream str; - str << "serialLockTestFile." << our_pid; + str << "serialLockTestFile." << our_pid << ".pid"; const std::string path = str.str(); OLA_ASSERT_FALSE_MSG(ola::io::FileExists(path), From a5281239b128f2f521ba5112d00461347c397cb2 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 20 Mar 2022 10:47:10 +0100 Subject: [PATCH 37/41] Update comments about serial port locking --- common/io/Serial.cpp | 2 ++ include/ola/io/Serial.h | 14 ++++++++------ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/common/io/Serial.cpp b/common/io/Serial.cpp index 3993a478f3..f244907e5c 100644 --- a/common/io/Serial.cpp +++ b/common/io/Serial.cpp @@ -308,6 +308,8 @@ bool AcquireLockAndOpenSerialPort(const std::string &path, int oflag, int *fd) { } #endif + // As a final safety mechanism, use ioctl(TIOCEXCL) if available to prevent + // further opens. if (!LockTIOCEXCL(*fd, path)) { close(*fd); return false; diff --git a/include/ola/io/Serial.h b/include/ola/io/Serial.h index e5cd84b760..36be9e010f 100644 --- a/include/ola/io/Serial.h +++ b/include/ola/io/Serial.h @@ -91,8 +91,10 @@ void ReleaseUUCPLock(const std::string &path); * @param[out] fd a pointer to the fd which is returned. * @returns true if the open succeeded, false otherwise. * - * Depending on the compile-time configuration, this will use either flock() - * (the default) or UUCP locking. See: ./configure --enable-uucp-locking. + * Depending on the compile-time configuration, this will use as many as + * possible out of the flock() system call, UUCP lockfiles and the TIOCEXCL + * ioctl. UUCP lockfiles are disabled by default, unless flock() is + * unavailable. See: ./configure --enable-uucp-locking. * * This fails-fast, it we can't get the lock immediately, we'll return false. * @@ -104,10 +106,10 @@ bool AcquireLockAndOpenSerialPort(const std::string &path, int oflag, int *fd); * @brief Release a lock for the serial port * @param path The path to unlock. * - * If UUCP locking was used (see AcquireLockAndOpenSerialPort()), the lockfile - * will be removed (but only if the PID matches). - * - * Does nothing if flock() was used (see ./configure --enable-uucp-locking). + * If UUCP locking was used (see AcquireLockAndOpenSerialPort()), and the PID + * in the lockfile matches our own, the lockfile will be removed. Otherwise, + * this call does nothing because the other locking methods do not require an + * explicit unlock. * * @see AcquireLockAndOpenSerialPort() */ From 98d4b66184019440dee6274d94348fc9dc2a3ca1 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 20 Mar 2022 10:49:47 +0100 Subject: [PATCH 38/41] Fix ordering of RemoveUUCPLockFile() and close() --- common/io/Serial.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/io/Serial.cpp b/common/io/Serial.cpp index f244907e5c..db185356a6 100644 --- a/common/io/Serial.cpp +++ b/common/io/Serial.cpp @@ -239,16 +239,16 @@ bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) { if (!TryOpen(path, oflag, fd)) { OLA_DEBUG << "Failed to open device " << path << " despite having the " << "lock file"; - ReleaseUUCPLock(path); close(*fd); + ReleaseUUCPLock(path); return false; } // As a final safety mechanism, use ioctl(TIOCEXCL) if available to prevent // further opens. if (!LockTIOCEXCL(*fd, path)) { - ReleaseUUCPLock(path); close(*fd); + ReleaseUUCPLock(path); return false; } From 64d4943da2740cd18af9428550dce4ae20944724 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 20 Mar 2022 10:56:40 +0100 Subject: [PATCH 39/41] Add more #endif comments --- common/io/Serial.cpp | 6 +++--- common/io/SerialLockTester.cpp | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/common/io/Serial.cpp b/common/io/Serial.cpp index db185356a6..6351d33e79 100644 --- a/common/io/Serial.cpp +++ b/common/io/Serial.cpp @@ -284,7 +284,7 @@ bool AcquireLockAndOpenSerialPort(const std::string &path, int oflag, int *fd) { if (!AcquireUUCPLock(path)) { return false; } -#endif +#endif // UUCP_LOCKING // Now try to open the serial device. if (!TryOpen(path, oflag, fd)) { @@ -294,7 +294,7 @@ bool AcquireLockAndOpenSerialPort(const std::string &path, int oflag, int *fd) { ReleaseUUCPLock(path); #else OLA_DEBUG << "Failed to open device " << path; -#endif +#endif // UUCP_LOCKING return false; } @@ -306,7 +306,7 @@ bool AcquireLockAndOpenSerialPort(const std::string &path, int oflag, int *fd) { } else { OLA_INFO << "Locked " << path << " using flock()"; } -#endif +#endif // HAVE_FLOCK // As a final safety mechanism, use ioctl(TIOCEXCL) if available to prevent // further opens. diff --git a/common/io/SerialLockTester.cpp b/common/io/SerialLockTester.cpp index d20d8f5daf..54a4ff0f92 100644 --- a/common/io/SerialLockTester.cpp +++ b/common/io/SerialLockTester.cpp @@ -72,9 +72,9 @@ void SerialLockTest::testLock() { // flock() is not available, but UUCP locking was selected. OK. #else OLA_FAIL("Not using UUCP locking, and flock() is not available"); -#endif +#endif // UUCP_LOCKING -#endif +#endif // HAVE_FLOCK close(fd); From 3ada1bd33f9fa7fbc6b6caae069f096431ce74e2 Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 20 Mar 2022 10:56:55 +0100 Subject: [PATCH 40/41] AcquireLockAndOpenSerialPort: Release UUCP lock on error paths --- common/io/Serial.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/common/io/Serial.cpp b/common/io/Serial.cpp index 6351d33e79..334125e93a 100644 --- a/common/io/Serial.cpp +++ b/common/io/Serial.cpp @@ -302,6 +302,9 @@ bool AcquireLockAndOpenSerialPort(const std::string &path, int oflag, int *fd) { if (flock(*fd, LOCK_EX | LOCK_NB) == -1) { OLA_INFO << "Failed to flock() device " << path; close(*fd); +#ifdef UUCP_LOCKING + ReleaseUUCPLock(path); +#endif // UUCP_LOCKING return false; } else { OLA_INFO << "Locked " << path << " using flock()"; @@ -312,6 +315,9 @@ bool AcquireLockAndOpenSerialPort(const std::string &path, int oflag, int *fd) { // further opens. if (!LockTIOCEXCL(*fd, path)) { close(*fd); +#ifdef UUCP_LOCKING + ReleaseUUCPLock(path); +#endif // UUCP_LOCKING return false; } From 227a6137a81560049603dab0770419ef8e61584c Mon Sep 17 00:00:00 2001 From: Thomas White Date: Sun, 13 Nov 2022 22:59:09 +0100 Subject: [PATCH 41/41] Apply suggestions from code review Co-authored-by: Peter Newman --- common/io/Makefile.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/io/Makefile.mk b/common/io/Makefile.mk index fb64291fce..8e480a677b 100644 --- a/common/io/Makefile.mk +++ b/common/io/Makefile.mk @@ -47,8 +47,8 @@ test_programs += \ common/io/MemoryBlockTester \ common/io/SelectServerTester \ common/io/StreamTester \ - common/io/TimeoutManagerTester \ - common/io/SerialLockTester + common/io/SerialLockTester \ + common/io/TimeoutManagerTester common_io_IOQueueTester_SOURCES = common/io/IOQueueTest.cpp common_io_IOQueueTester_CXXFLAGS = $(COMMON_TESTING_FLAGS)