From 19da18b96897c7d0b9dce2dd589d0135b397005e Mon Sep 17 00:00:00 2001 From: Denis Shienkov Date: Tue, 3 Jun 2014 22:06:31 +0400 Subject: Refactor the availablePortsByUdev() in favor to readability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implementation of the availablePortsByUdev() function is a little complicated by excess "if/else" conditions and also too long lines, that worsens readability. It is reasonable to make the following: * To get rid of the big "if {...}" blocks in favor to immediate return from function in case of an error. It will allow to reduce a quantity of lines and will shift code alignment to the left. * To split declaration of some long variables and functions into separate lines, with length at least up to 80~100 characters. * To drop of the 'struct' keywords and the '::' global namespace operator for variables. Tested on ArchLinux 64 bit with the on-board and PL2303 serial ports using Qt4 and then Qt5. Tested build on Android x86 using Qt5. Change-Id: Iddc2a9511230e56e4a9d01a4c22af7b2eaeae60c Reviewed-by: Peter Kümmel Reviewed-by: Sergey Belyashov Reviewed-by: Denis Shienkov --- src/serialport/qserialportinfo_unix.cpp | 86 +++++++++++++++------------------ 1 file changed, 38 insertions(+), 48 deletions(-) diff --git a/src/serialport/qserialportinfo_unix.cpp b/src/serialport/qserialportinfo_unix.cpp index 0529973..b4caf88 100644 --- a/src/serialport/qserialportinfo_unix.cpp +++ b/src/serialport/qserialportinfo_unix.cpp @@ -296,72 +296,62 @@ QList availablePortsByUdev() if (!symbolsResolved) return QList(); #endif - QList serialPortInfoList; - static const QString rfcommDeviceName(QStringLiteral("rfcomm")); QScopedPointer udev(::udev_new()); - if (udev) { - - QScopedPointer enumerate(::udev_enumerate_new(udev.data())); - - if (enumerate) { - - ::udev_enumerate_add_match_subsystem(enumerate.data(), "tty"); - ::udev_enumerate_scan_devices(enumerate.data()); - - struct ::udev_list_entry *devices = - ::udev_enumerate_get_list_entry(enumerate.data()); - - struct ::udev_list_entry *dev_list_entry; - udev_list_entry_foreach(dev_list_entry, devices) { - - QScopedPointer dev(::udev_device_new_from_syspath(udev.data(), - ::udev_list_entry_get_name(dev_list_entry))); - - if (dev) { + if (!udev) + return QList(); - QSerialPortInfo serialPortInfo; + QScopedPointer + enumerate(::udev_enumerate_new(udev.data())); - serialPortInfo.d_ptr->device = QString::fromLatin1(::udev_device_get_devnode(dev.data())); - serialPortInfo.d_ptr->portName = QString::fromLatin1(::udev_device_get_sysname(dev.data())); + if (!enumerate) + return QList(); - struct ::udev_device *parentdev = ::udev_device_get_parent(dev.data()); + ::udev_enumerate_add_match_subsystem(enumerate.data(), "tty"); + ::udev_enumerate_scan_devices(enumerate.data()); - if (parentdev) { + udev_list_entry *devices = ::udev_enumerate_get_list_entry(enumerate.data()); - if (checkUdevForSerial8250Driver(parentdev)) - continue; + QList serialPortInfoList; + udev_list_entry *dev_list_entry; + udev_list_entry_foreach(dev_list_entry, devices) { - serialPortInfo.d_ptr->description = getUdevModelName(dev.data()); - serialPortInfo.d_ptr->manufacturer = getUdevVendorName(dev.data()); - serialPortInfo.d_ptr->serialNumber = getUdevSerialNumber(dev.data()); + QScopedPointer + dev(::udev_device_new_from_syspath( + udev.data(), ::udev_list_entry_get_name(dev_list_entry))); - serialPortInfo.d_ptr->vendorIdentifier = getUdevModelIdentifier( - dev.data(), serialPortInfo.d_ptr->hasVendorIdentifier); + if (!dev) + return serialPortInfoList; - serialPortInfo.d_ptr->productIdentifier = getUdevVendorIdentifier( - dev.data(), serialPortInfo.d_ptr->hasProductIdentifier); - } else { - if (serialPortInfo.d_ptr->portName.startsWith(rfcommDeviceName)) { - bool ok; - int portNumber = serialPortInfo.d_ptr->portName.mid(rfcommDeviceName.length()).toInt(&ok); + QSerialPortInfo serialPortInfo; - if (!ok || (portNumber < 0) || (portNumber > 255)) - continue; - } else { - continue; - } - } + serialPortInfo.d_ptr->device = QString::fromLatin1(::udev_device_get_devnode(dev.data())); + serialPortInfo.d_ptr->portName = QString::fromLatin1(::udev_device_get_sysname(dev.data())); - serialPortInfoList.append(serialPortInfo); - } + udev_device *parentdev = ::udev_device_get_parent(dev.data()); + if (parentdev) { + if (checkUdevForSerial8250Driver(parentdev)) + continue; + serialPortInfo.d_ptr->description = getUdevModelName(dev.data()); + serialPortInfo.d_ptr->manufacturer = getUdevVendorName(dev.data()); + serialPortInfo.d_ptr->serialNumber = getUdevSerialNumber(dev.data()); + serialPortInfo.d_ptr->vendorIdentifier = getUdevModelIdentifier(dev.data(), serialPortInfo.d_ptr->hasVendorIdentifier); + serialPortInfo.d_ptr->productIdentifier = getUdevVendorIdentifier(dev.data(), serialPortInfo.d_ptr->hasProductIdentifier); + } else { + if (serialPortInfo.d_ptr->portName.startsWith(rfcommDeviceName)) { + bool ok; + int portNumber = serialPortInfo.d_ptr->portName.mid(rfcommDeviceName.length()).toInt(&ok); + if (!ok || (portNumber < 0) || (portNumber > 255)) + continue; + } else { + continue; } - } + serialPortInfoList.append(serialPortInfo); } return serialPortInfoList; -- cgit v1.2.1