Review Request 130090: Fix incorrect definition of major(3)/minor(3) macros

[KJ Tsanaktsidis]

> On April 20, 2017, 4:19 p.m., Lamarque Souza wrote:
> > autotests/fakeUdisks2.h, line 35
> > <https://git.reviewboard.kde.org/r/130090/diff/4/?file=494486#file494486line35>
> >
> >     It is common practice to pass QString parameters QString as const &:
> >     
> >     FakeUdisks2BlockDevice(QObject *parent, const &QString device, quint64 device_number);

Sorry - you've probably noticed by now but my C++ is pretty rusty... (no pun intended)


- KJ


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/130090/#review103071
-----------------------------------------------------------


On April 22, 2017, 12:38 a.m., KJ Tsanaktsidis wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/130090/
> -----------------------------------------------------------
> 
> (Updated April 22, 2017, 12:38 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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170422/bf415da1/attachment.html>