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 <thiago.macieira@intel.com>
This commit is contained in:
Marc Mutz 2016-09-15 17:31:25 +02:00
parent 76adb6c29f
commit 29d64bc8e0
2 changed files with 33 additions and 18 deletions

View File

@ -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<char *>(buffer) + n, sizeof(buffer) - n);
int result = qt_safe_read(m_fd.get(), reinterpret_cast<char *>(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);

View File

@ -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