Fix incorrect definition of major(3)/minor(3) macros

Review Request #130090 - Created April 17, 2017 and updated

KJ Tsanaktsidis
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).

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.


  • 1
  • 12
  • 0
  • 13
Description From Last Updated
I think registering to systemBus only works if you are root or has the right permissions in dbus' configuration. Have ... Lamarque Souza Lamarque Souza
Lamarque Souza
KJ Tsanaktsidis
KJ Tsanaktsidis
Lamarque Souza
KJ Tsanaktsidis
Lamarque Souza
KJ Tsanaktsidis
Lamarque Souza
KJ Tsanaktsidis
Review request changed

Change Summary:

  • Fix definition style of const-returning functions
  • Replace if()..elseif()...else() CMake macros with a clearer control flow




Revision 6 (+256 -10)

Show changes