From 29d64bc8e06d6809ac0c68b7b5459a8a51667769 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Thu, 15 Sep 2016 17:31:25 +0200 Subject: QEvdevKeyboardHandler: use RAII in create()/ctor Coverity somewhat rightfully complained that the FD may be leaked in certain cases, e.g. when the new-expression throws. Yes, the plugin is compiled with exceptions disabled, but the code is still a bug waiting to happen, because it's too easy to just add an early return to the function and leak the FD that way. Fix by writing a small RAII class for FDs (can't use QSharedPointer, since it's not a pointer we're dealing with). It's quite generically named, in anticipation that it might come in handy elsewhere, too. Coverity-Id: 89046 Change-Id: I83d1ed3f11219065d2248c129ed191a651f617c7 Reviewed-by: Thiago Macieira --- .../input/evdevkeyboard/qevdevkeyboardhandler.cpp | 34 ++++++++++++---------- .../input/evdevkeyboard/qevdevkeyboardhandler_p.h | 17 +++++++++-- 2 files changed, 33 insertions(+), 18 deletions(-) (limited to 'src') diff --git a/src/platformsupport/input/evdevkeyboard/qevdevkeyboardhandler.cpp b/src/platformsupport/input/evdevkeyboard/qevdevkeyboardhandler.cpp index 089cc13032..3363859dae 100644 --- a/src/platformsupport/input/evdevkeyboard/qevdevkeyboardhandler.cpp +++ b/src/platformsupport/input/evdevkeyboard/qevdevkeyboardhandler.cpp @@ -52,8 +52,15 @@ Q_LOGGING_CATEGORY(qLcEvdevKeyMap, "qt.qpa.input.keymap") // simple builtin US keymap #include "qevdevkeyboard_defaultmap_p.h" -QEvdevKeyboardHandler::QEvdevKeyboardHandler(const QString &device, int fd, bool disableZap, bool enableCompose, const QString &keymapFile) - : m_device(device), m_fd(fd), m_notify(Q_NULLPTR), +void QFdContainer::reset() Q_DECL_NOTHROW +{ + if (m_fd >= 0) + qt_safe_close(m_fd); + m_fd = -1; +} + +QEvdevKeyboardHandler::QEvdevKeyboardHandler(const QString &device, QFdContainer &fd, bool disableZap, bool enableCompose, const QString &keymapFile) + : m_device(device), m_fd(fd.release()), m_notify(Q_NULLPTR), m_modifiers(0), m_composing(0), m_dead_unicode(0xffff), m_no_zap(disableZap), m_do_compose(enableCompose), m_keymap(0), m_keymap_size(0), m_keycompose(0), m_keycompose_size(0) @@ -68,16 +75,13 @@ QEvdevKeyboardHandler::QEvdevKeyboardHandler(const QString &device, int fd, bool unloadKeymap(); // socket notifier for events on the keyboard device - m_notify = new QSocketNotifier(m_fd, QSocketNotifier::Read, this); + m_notify = new QSocketNotifier(m_fd.get(), QSocketNotifier::Read, this); connect(m_notify, SIGNAL(activated(int)), this, SLOT(readKeycode())); } QEvdevKeyboardHandler::~QEvdevKeyboardHandler() { unloadKeymap(); - - if (m_fd >= 0) - qt_safe_close(m_fd); } QEvdevKeyboardHandler *QEvdevKeyboardHandler::create(const QString &device, @@ -111,13 +115,12 @@ QEvdevKeyboardHandler *QEvdevKeyboardHandler::create(const QString &device, qCDebug(qLcEvdevKey) << "Opening keyboard at" << device; - int fd; - fd = qt_safe_open(device.toLocal8Bit().constData(), O_RDONLY | O_NDELAY, 0); - if (fd >= 0) { - ::ioctl(fd, EVIOCGRAB, grab); + QFdContainer fd(qt_safe_open(device.toLocal8Bit().constData(), O_RDONLY | O_NDELAY, 0)); + if (fd.get() >= 0) { + ::ioctl(fd.get(), EVIOCGRAB, grab); if (repeatDelay > 0 && repeatRate > 0) { int kbdrep[2] = { repeatDelay, repeatRate }; - ::ioctl(fd, EVIOCSREP, kbdrep); + ::ioctl(fd.get(), EVIOCSREP, kbdrep); } return new QEvdevKeyboardHandler(device, fd, disableZap, enableCompose, keymapFile); @@ -137,7 +140,7 @@ void QEvdevKeyboardHandler::switchLed(int led, bool state) led_ie.code = led; led_ie.value = state; - qt_safe_write(m_fd, &led_ie, sizeof(led_ie)); + qt_safe_write(m_fd.get(), &led_ie, sizeof(led_ie)); } void QEvdevKeyboardHandler::readKeycode() @@ -146,7 +149,7 @@ void QEvdevKeyboardHandler::readKeycode() int n = 0; forever { - int result = qt_safe_read(m_fd, reinterpret_cast(buffer) + n, sizeof(buffer) - n); + int result = qt_safe_read(m_fd.get(), reinterpret_cast(buffer) + n, sizeof(buffer) - n); if (result == 0) { qWarning("evdevkeyboard: Got EOF from the input device"); @@ -159,8 +162,7 @@ void QEvdevKeyboardHandler::readKeycode() if (errno == ENODEV) { delete m_notify; m_notify = Q_NULLPTR; - qt_safe_close(m_fd); - m_fd = -1; + m_fd.reset(); } return; } @@ -471,7 +473,7 @@ void QEvdevKeyboardHandler::unloadKeymap() //Set locks according to keyboard leds quint16 ledbits[1]; memset(ledbits, 0, sizeof(ledbits)); - if (::ioctl(m_fd, EVIOCGLED(sizeof(ledbits)), ledbits) < 0) { + if (::ioctl(m_fd.get(), EVIOCGLED(sizeof(ledbits)), ledbits) < 0) { qWarning("evdevkeyboard: Failed to query led states"); switchLed(LED_NUML,false); switchLed(LED_CAPSL, false); diff --git a/src/platformsupport/input/evdevkeyboard/qevdevkeyboardhandler_p.h b/src/platformsupport/input/evdevkeyboard/qevdevkeyboardhandler_p.h index 84c251c3c2..b08f30b6ee 100644 --- a/src/platformsupport/input/evdevkeyboard/qevdevkeyboardhandler_p.h +++ b/src/platformsupport/input/evdevkeyboard/qevdevkeyboardhandler_p.h @@ -123,12 +123,25 @@ inline QDataStream &operator<<(QDataStream &ds, const QEvdevKeyboardMap::Composi return ds << c.first << c.second << c.result; } +class QFdContainer +{ + int m_fd; + Q_DISABLE_COPY(QFdContainer); +public: + explicit QFdContainer(int fd = -1) Q_DECL_NOTHROW : m_fd(fd) {} + ~QFdContainer() { reset(); } + + int get() const Q_DECL_NOTHROW { return m_fd; } + + int release() Q_DECL_NOTHROW { int result = m_fd; m_fd = -1; return result; } + void reset() Q_DECL_NOTHROW; +}; class QEvdevKeyboardHandler : public QObject { Q_OBJECT public: - QEvdevKeyboardHandler(const QString &device, int fd, bool disableZap, bool enableCompose, const QString &keymapFile); + QEvdevKeyboardHandler(const QString &device, QFdContainer &fd, bool disableZap, bool enableCompose, const QString &keymapFile); ~QEvdevKeyboardHandler(); enum KeycodeAction { @@ -181,7 +194,7 @@ private: void switchLed(int, bool); QString m_device; - int m_fd; + QFdContainer m_fd; QSocketNotifier *m_notify; // keymap handling -- cgit v1.2.3