----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/130090/#review103148 -----------------------------------------------------------
Ship it! Ship It! - Lamarque Souza On April 23, 2017, 9:56 a.m., KJ Tsanaktsidis wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/130090/ > ----------------------------------------------------------- > > (Updated April 23, 2017, 9:56 a.m.) > > > Review request for KDE Frameworks. > > > Repository: solid > > > Description > ------- > > Previously, udesksblock.cpp was attempting to find a definition for > major/minor on Linux in <sys/kdev_t.h> by checking Q_OS_LINUX before > importing the header. Q_OS_LINUX is however only set when > qsystemdetection.h is included, and the macro was being checked first. > > Even had this check worked, it would still be wrong. On a modern version > of the userspace linux-headers, <sys/kdev_t.h> includes definitions for > major and minor that assume each is limited to 8 bits and that dev_t is > 16 bits. This is no longer true anymore; on Linux, major numbers can be > up to 12 bits at present and minor numbers up to 20. Calling these > macros with dev_t values > 2^16 would give incorrect results. > > Because the Q_OS_LINUX check failed, a fallback version of the macros > were defined for use on all platforms. The code is allegedly copied from > kdev_t.h, except it is copied from the *kernel* version of the header, > not the userspace version. Linux internally uses a different > representation of dev_t than it exposes to userspace - the kernelspace > version is 20 bits of minor/12 bits of major contiguously, but the > userspace version packs the bits in a different order to maintain > compatability with old 16-bit device numbers. Thus, this code also does > not work for dev_t values > 2^16. > > To fix this, we add CMake rules to search for a system-provided > definition of the major/minor macros - on various systems, these can be > in a few different places. As a fallback, we assume old-style 16-bit > dev_t (although I suspect that is only used for Windows, where > major/minor numbers are pretty meaningless anyway). > > > Diffs > ----- > > autotests/CMakeLists.txt 54adeea62b954b9169b37f1eab8fa3e215fafafa > autotests/fakeUdisks2.h PRE-CREATION > autotests/fakeUdisks2.cpp PRE-CREATION > autotests/solidudisks2test.cpp PRE-CREATION > src/solid/devices/backends/udisks2/CMakeLists.txt > 34390064af29ace07cbb3470945be098cc606d04 > src/solid/devices/backends/udisks2/udisksblock.cpp > 0622ec77fcf670a2005d34b7a6c31ca8b53a18d8 > > Diff: https://git.reviewboard.kde.org/r/130090/diff/ > > > Testing > ------- > > I've written a little snippet to iterate through block devices, print their > major/minor number, and their device properties. It was previously > incorrectly labeling all my disks with major 0 and minor == device_number > (since it was using the first 20 bits for the minor). It now correctly > identifies their major/minor number. > > > Thanks, > > KJ Tsanaktsidis > >